netdev - Re: [PATCH 3/5] net: Set fput_needed iff FDPUT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 6 Aug 2020 13:08:58 +0100
From:   Al Viro <viro@...iv.linux.org.uk>
To:     linmiaohe <linmiaohe@...wei.com>
Cc:     davem@...emloft.net, kuba@...nel.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/5] net: Set fput_needed iff FDPUT_FPUT is set

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@...wei.com>
> > 
> > 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.