netdev - Re: [PATCH 1/2] Revert "9p: p9_client_create: use p9_client

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <YzyfM1rJrmT1Qe4N@codewreck.org>
Date:   Wed, 5 Oct 2022 06:01:39 +0900
From:   Dominique Martinet <asmadeus@...ewreck.org>
To:     Dan Carpenter <dan.carpenter@...cle.com>
Cc:     Leon Romanovsky <leon@...nel.org>,
        v9fs-developer@...ts.sourceforge.net, linux_oss@...debyte.com,
        linux-kernel@...r.kernel.org, syzkaller-bugs@...glegroups.com,
        syzbot+67d13108d855f451cafc@...kaller.appspotmail.com,
        davem@...emloft.net, edumazet@...gle.com, ericvh@...il.com,
        kuba@...nel.org, lucho@...kov.net, netdev@...r.kernel.org
Subject: Re: [PATCH 1/2] Revert "9p: p9_client_create: use p9_client_destroy
 on failure"

Dan Carpenter wrote on Tue, Oct 04, 2022 at 04:10:44PM +0300:
> On Thu, Sep 29, 2022 at 12:37:55PM +0300, Leon Romanovsky wrote:
> > Rely on proper unwind order.
> > 
> > This reverts commit 3ff51294a05529d0baf6d4b2517e561d12efb9f9.
> > 
> > Reported-by: syzbot+67d13108d855f451cafc@...kaller.appspotmail.com
> > Signed-off-by: Leon Romanovsky <leon@...nel.org>
> 
> The commit message doesn't really say what the problem is to the user.
> Is this just to make the next patch easier?

Yes (and perhaps a bit of spite from the previous discussion), and the
next patch was not useable so I am not applying this as is.

The next patch was meant as an alternative implementation to this fix:
https://lore.kernel.org/all/20220928221923.1751130-1-asmadeus@codewreck.org/T/#u

At this point I have the original fix in my -next branch but it hasn't
had any positive review (and well, I myself agree it's ugly), so unless
Leon sends a v2 I'll need to think of a better way of tracking if
clnt->trans_mod->create has been successfully called.
I'm starting to think that since we don't have so many clnt I can
probably just fit a bool/bitfield in one of the holes of the struct
and just keep track of it; that'll be less error-prone than relying on
clnt->trans (which -is- initialized in create() most of the time, but
not in a way we can rely on) or reworking create() to return it as I
originally wanted to do (rdma still needs to populate clnt->trans behind
the scenees before create() returns, so the abstraction is also quite
ugly)

The current breakage is actually quite bad so I'll try to send that
today or tomorrow for merging next week unless Leon resends something we
can work with... Conceptually won't be different than the patch
currently in next so hopefully can pretend it's had a couple of weeks of
testing...
-- 
Dominique