netdev - Re: [PATCH net-next v1] net/9p/trans_fd: check the return value of parse

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 linux-cve-announce PHC
Open Source and information security mailing list archives

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

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

Date:   Thu, 22 Sep 2022 11:38:20 +0200
From:   Christian Schoenebeck <linux_oss@...debyte.com>
To:     asmadeus@...ewreck.org, Li Zhong <floridsleeves@...il.com>
Cc:     netdev@...r.kernel.org, v9fs-developer@...ts.sourceforge.net,
        pabeni@...hat.com, kuba@...nel.org, edumazet@...gle.com,
        davem@...emloft.net, lucho@...kov.net, ericvh@...il.com
Subject: Re: [PATCH net-next v1] net/9p/trans_fd: check the return value of parse_opts

On Donnerstag, 22. September 2022 00:12:38 CEST Li Zhong wrote:
> On Wed, Sep 21, 2022 at 2:23 PM <asmadeus@...ewreck.org> wrote:
> > Li Zhong wrote on Wed, Sep 21, 2022 at 02:09:21PM -0700:
> > > parse_opts() could fail when there is error parsing mount options into
> > > p9_fd_opts structure due to allocation failure. In that case opts will
> > > contain invalid data.
> > 
> > In practice opts->rfd/wfd is set to ~0 before the failure modes so they
> > will contain exactly what we want them to contain: something that'll
> > fail the check below.
> > 
> > It is however cleared like this so I'll queue this patch in 9p tree when
> > I have a moment, but I'll clarify the commit message to say this is
> > NO-OP : please feel free to send a v2 if you want to put your own words
> > in there; otherwise it'll be something like below:
> > ----
> > net/9p: clarify trans_fd parse_opt failure handling
> > 
> > This parse_opts will set invalid opts.rfd/wfd in case of failure which
> > we already check, but it is not clear for readers that parse_opts error
> > are handled in p9_fd_create: clarify this by explicitly checking the
> > return value.
> > ----
> 
> Thanks for the patient reply! I agree that the check on
> opts.rfd/wft against ~0 will prevent error even if it fails
> memory allocation. But currently the error log is
> 'insufficient options', which is kind of misleading and the
> error code returned is -ENOPROTOOPT instead of -ENOMEM, which
> I guess would be better if we distinguish between them.

Avoiding those confusions for users makes sense to me, but then please also 
mention that in the commit log, because it is also useful to know the actual 
motivation of a patch.

Best regards,
Christian Schoenebeck