linux-kernel - RE: [PATCH RFC 4/4] 9p: fix race issue in fid contention.

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: <HE1PR0802MB255560720A13BD59C11DEA00F43F0@HE1PR0802MB2555.eurprd08.prod.outlook.com>
Date:   Fri, 18 Sep 2020 08:57:22 +0000
From:   Jianyong Wu <Jianyong.Wu@....com>
To:     Dominique Martinet <asmadeus@...ewreck.org>
CC:     "ericvh@...il.com" <ericvh@...il.com>,
        "lucho@...kov.net" <lucho@...kov.net>,
        "v9fs-developer@...ts.sourceforge.net" 
        <v9fs-developer@...ts.sourceforge.net>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Justin He <Justin.He@....com>, Greg Kurz <groug@...d.org>
Subject: RE: [PATCH RFC 4/4] 9p: fix race issue in fid contention.

Hi Dominique,

> -----Original Message-----
> From: Dominique Martinet <asmadeus@...ewreck.org>
> Sent: Monday, September 14, 2020 4:32 PM
> To: Jianyong Wu <Jianyong.Wu@....com>
> Cc: ericvh@...il.com; lucho@...kov.net; v9fs-
> developer@...ts.sourceforge.net; linux-kernel@...r.kernel.org; Justin He
> <Justin.He@....com>; Greg Kurz <groug@...d.org>
> Subject: Re: [PATCH RFC 4/4] 9p: fix race issue in fid contention.
>
> Jianyong Wu wrote on Mon, Sep 14, 2020:
> > > Not having exceptions for that will also make the code around
> > > fid_atomic_dec much simpler: just have clunk do an atomic dec and
> > > only do the actual clunk if that hit zero, and we should be able to
> > > get rid of that helper?
> >
> > Sorry, I think always-one refcount  won't work at this point, as the
> > fid will be clunked only by file context itself not the every consumer
> > of every fid. We can't decrease the refcounter at just one static
> > point.
> > Am I wrong?
>
> I don't understand the "We can't decrease the refcounter at just one static
> point".
> Basically everywhere you added a fid_atomic_dec() will just need to be
> changed to clunk -- the basic rule of refcounting is that anywhere you take a
> ref you need to put it back.
>
> All these places take a fid to do some RPC already so it's not a problem to add
> a clunk and do one more; especially since the "clunk" will just be just a deref.
> For consistency I'd advocate for the kref API as we use that for requests
> already; it might be better to rename the clunk calls to p9_fid_put or
> something but I think that's more churn than it's worth....
>
If we move the counter decrease code into p9_client_clunk and put it instead of fid_atomic_dec, we need delete fid off the inode where it stores in p9_client_clunk.
But there is no way can we acquire the inode in p9_client_clunk. Do you have any idea? I think introduce another parameter for p9_client_clunk
Is not graceful.

Thanks
Jianyong Wu
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.