linux-kernel - Re: [RFC PATCH] fpga: remove module reference counting from core components

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: <2023110839-jam-relapsing-7f5d@gregkh>
Date:   Wed, 8 Nov 2023 17:20:53 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Xu Yilun <yilun.xu@...ux.intel.com>
Cc:     Marco Pagani <marpagan@...hat.com>,
        Moritz Fischer <mdf@...nel.org>, Wu Hao <hao.wu@...el.com>,
        Xu Yilun <yilun.xu@...el.com>, Tom Rix <trix@...hat.com>,
        Alan Tull <atull@...nsource.altera.com>,
        linux-fpga@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] fpga: remove module reference counting from core
 components

On Wed, Nov 08, 2023 at 11:52:52PM +0800, Xu Yilun wrote:
> > >>
> > >> In fpga_region_get() / fpga_region_put(): call get_device() before
> > >> acquiring the mutex and put_device() after having released the mutex
> > >> to avoid races.

Why do you need another reference count with a lock?  You already have
that with the calls to get/put_device().

> > > Could you help elaborate more about the race?
> > > 
> > 
> > I accidentally misused the word race. My concern was that memory might
> > be released after the last put_device(), causing mutex_unlock() to be
> > called on a mutex that does not exist anymore. It should not happen
> > for the moment since the region does not use devres, but I think it
> > still makes the code more brittle.
> 
> It makes sense.
> 
> But I dislike the mutex itself. The purpose is to exclusively grab the
> device, but a mutex is too much heavy for this.

Why "heavy"?  Is this a fast-path?  Have you measured it?

> The lock/unlock of mutex
> scattered in different functions is also an uncommon style. Maybe some
> atomic count should be enough.

Don't make a fake lock with an atomic variable, use real locks if you
need it.

Or don't even care about module unloading at all!  Why does it matter?
It can never happen without explicit intervention and it's something
that a lot of the time, just will cause problems.  Don't do a lot of
extra work for something that doesn't matter.

thanks,

greg k-h