Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 27 Dec 2019 12:24:13 -0500 (EST)
From:   Vladis Dronov <vdronov@redhat.com>
To:     Richard Cochran <richardcochran@gmail.com>
Cc:     linux-fsdevel@vger.kernel.org,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        Al Viro <aviro@redhat.com>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Message-ID: <1031316500.2657655.1577467453350.JavaMail.zimbra@redhat.com>
In-Reply-To: <20191227150218.GA1435@localhost>
References: <20191208195340.GX4203@ZenIV.linux.org.uk> <20191227022627.24476-1-vdronov@redhat.com> <20191227150218.GA1435@localhost>
Subject: Re: [PATCH v2] ptp: fix the race between the release of ptp_clock
 and cdev
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Thread-Topic: fix the race between the release of ptp_clock and cdev
Thread-Index: irWUFwtSG/M8AhEHYaeQCeWyBgqSgA==
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hello, Richard,

Thank you for the review!

> > + * @dev:   Pointer to the initialized device. Caller must provide
> > + *         'release' filed
> 
> field

Indeed. *sigh* Nothing is ideal. Let's hope a maintainer could fix it if
this is approved.

Best regards,
Vladis Dronov | Red Hat, Inc. | The Core Kernel | Senior Software Engineer

----- Original Message -----
> From: "Richard Cochran" <richardcochran@gmail.com>
> To: "Vladis Dronov" <vdronov@redhat.com>
> Cc: linux-fsdevel@vger.kernel.org, "Alexander Viro" <viro@zeniv.linux.org.uk>, "Al Viro" <aviro@redhat.com>,
> netdev@vger.kernel.org, linux-kernel@vger.kernel.org
> Sent: Friday, December 27, 2019 4:02:19 PM
> 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@gmail.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