linux-kernel - Re: [PATCH 2/4] cxl/mem: Fix cdev_device

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]

Date:   Mon, 29 Mar 2021 19:44:39 -0300
From:   Jason Gunthorpe <jgg@...dia.com>
To:     Dan Williams <dan.j.williams@...el.com>
Cc:     linux-cxl@...r.kernel.org, "Weiny, Ira" <ira.weiny@...el.com>,
        Vishal L Verma <vishal.l.verma@...el.com>,
        "Schofield, Alison" <alison.schofield@...el.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/4] cxl/mem: Fix cdev_device_add() error handling

On Mon, Mar 29, 2021 at 02:03:37PM -0700, Dan Williams wrote:

> Ugh, exactly why I was motivated to attempt to preclude this with new
> core infrastructure that attempted to fix this centrally [1]. Remove
> the  possibility of "others" getting this wrong. However after my
> initial idea bounced off Greg then I ended up shipping this bug in the
> local rewrite. I think the debugfs api gets this right in terms of
> centralizing the reference count management, and I want to see
> something similar for common driver ioctl patterns.

There is a lot of variety here, I'm not sure how much valuable common
code there will be that could be lifted into the core.. srcu,
refcount, rwsem, percpu_ref, etc are all common implementations with
various properties.

The easist implementation is to just block driver destruction with a
refcount & completion pattern

The hardest is to allow the underlying HW driver to be removed from
the fops while the file remains open.

Usually whatever scheme is used has to flow into some in-kernel API as
well, so isolating it in cdev may no be entirely helpful.

The easisted helper API would be to add an 'unregistration lock' to
the struct device that blocks unregistration. A refcount & completion
for instance. I've seen that open coded enough times.

Jason