linux-kernel - Re: [PATCH v2] ptp: fix the race between the release of ptp

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: <20191227150218.GA1435@localhost>
Date:   Fri, 27 Dec 2019 07:02:19 -0800
From:   Richard Cochran <richardcochran@...il.com>
To:     Vladis Dronov <vdronov@...hat.com>
Cc:     linux-fsdevel@...r.kernel.org,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Al Viro <aviro@...hat.com>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] ptp: fix the race between the release of ptp_clock
 and cdev

On Fri, Dec 27, 2019 at 03:26:27AM +0100, Vladis Dronov wrote:
> Here cdev is embedded in posix_clock which is embedded in ptp_clock.
> The race happens because ptp_clock's lifetime is controlled by two
> refcounts: kref and cdev.kobj in posix_clock. This is wrong.
> 
> Make ptp_clock's sysfs device a parent of cdev with cdev_device_add()
> created especially for such cases. This way the parent device with its
> ptp_clock is not released until all references to the cdev are released.
> This adds a requirement that an initialized but not exposed struct
> device should be provided to posix_clock_register() by a caller instead
> of a simple dev_t.
> 
> This approach was adopted from the commit 72139dfa2464 ("watchdog: Fix
> the race between the release of watchdog_core_data and cdev"). See
> details of the implementation in the commit 233ed09d7fda ("chardev: add
> helper function to register char devs with a struct device").

Thanks for digging into this!

Acked-by: Richard Cochran <richardcochran@...il.com>

>  /**
>   * posix_clock_register() - register a new clock
> - * @clk:   Pointer to the clock. Caller must provide 'ops' and 'release'
> - * @devid: Allocated device id
> + * @clk:   Pointer to the clock. Caller must provide 'ops' field
> + * @dev:   Pointer to the initialized device. Caller must provide
> + *         'release' filed

field

Thanks,
Richard