Re: [PATCH 3/5] net: Set fput_needed iff FDPUT_FPUT is set

From: Al Viro
Date: Thu Aug 06 2020 - 13:25:00 EST


On Thu, Aug 06, 2020 at 12:59:16PM +0100, Al Viro wrote:
> On Thu, Aug 06, 2020 at 07:53:16PM +0800, linmiaohe wrote:
> > From: Miaohe Lin <linmiaohe@xxxxxxxxxx>
> >
> > We should fput() file iff FDPUT_FPUT is set. So we should set fput_needed
> > accordingly.
> >
> > Fixes: 00e188ef6a7e ("sockfd_lookup_light(): switch to fdget^W^Waway from fget_light")
>
> Explain, please. We are getting it from fdget(); what else can we get in flags there?

FWIW, struct fd ->flags may have two bits set: FDPUT_FPUT and FDPUT_POS_UNLOCK.
The latter is set only by __fdget_pos() and its callers, and that only for
regular files and directories.

Nevermind that sockfd_lookup_light() does *not* use ..._pos() family of
primitives, even if it started to use e.g. fdget_pos() it *still* would
not end up with anything other than FDPUT_FPUT to deal with on that
path - it checks that what it got is a socket. Anything else is dropped
right there, without leaving fput() to caller.

So could you explain what exactly the bug is - if you are seeing some
breakage and this patch fixes it, something odd is definitely going on
and it would be nice to figure out what that something is.