Re: [GIT PULL] gpio: updates for v5.13
From: Bartosz Golaszewski
Date: Wed May 05 2021 - 10:20:11 EST
On Tue, May 4, 2021 at 7:34 PM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> On Tue, May 04, 2021 at 04:17:02PM +0200, Bartosz Golaszewski wrote:
> > > Incidentally, if your code critically depends upon some field
> > > being first in such-and-such structure, you should either get rid of
> > > the dependency or at least bother to document that.
> > > That
> > > + /*
> > > + * Free memory allocated for the pending and live
> > > directories
> > > + * of committable groups.
> > > + */
> > > + if (sd->s_type & (CONFIGFS_GROUP_PENDING |
> > > CONFIGFS_GROUP_LIVE))
> > > + kfree(sd->s_element);
> > > +
> > > is asking for trouble down the road.
> > >
> > I'm not sure if this is a hard NAK for these changes or if you
> > consider this something that can be ironed out post v5.13-rc1?
> Rename implementation is simply bogus. You are, for some reason, attaching
> stuff to *destination*, which won't be seen by anyone not currently using
> it. It's the old_dentry that will be seen from that point on - you are
> moving it to new location by that d_move(). So I rather wonder how much
> had that thing been tested. And I'm pretty much certain that you are
> mishandling the refcounts on configfs-internal objects, with everything
> that entails in terms of UAF and leaks.
The interface's stability in user-space has been tested a lot with the
test-suite for libgpiod but I didn't look for leaks indeed.
> FWIW, I'm not happy about the userland API of that thing (what is supposed
> to happen if you create, move to live, then create another with the same
> name and try to move it to live or original back from live?), but
> Documentation/filesystems/configfs.rst is too sparse on such details.
> So I would like to see the specifics on that as well. _Before_ signing
> up on anything, including "we can fix it up after merge".
Understood. I've sent out a new PR without these changes. I'll start
another thread asking for your help on the correct approach and maybe
some better ideas for the user interface.