Re: [PATCH 0/7] 3.8-rc regression with pps-ldisc due to 70ece7a731

Re: [PATCH 0/7] 3.8-rc regression with pps-ldisc due to 70ece7a731

[Greg KH]

On Fri, Feb 08, 2013 at 02:05:40AM -0500, George Spelvin wrote:
> The standard N_TTY line discipline used to not use the tty->disc_data
> field, so N_PPS felt free to use it.  That has now changed, requiring
> that N_PPS use a different method to find its private data.
> 
> (In the current, buggy, state, N_PPS follows a wild pointer and explodes
> in an interrupt hander as soon as a pulse actually arrives.)
> 
> Strictly speaking, only patches 5 and 6 are the bugfix proper; the
> rest are various cleanups enabled by the looser coupling of the
> line disciplines.

As this is a breakage in 3.8-rc1, is there any way to pull out patches 5
and 6 in a format that I can apply them now, or in a way that I can get
them into 3.8.1 so that this gets fixed?  I can't apply all of these to
3.8-final now, as it's way too late.

thanks,

greg k-h

Re: [PATCH 0/7] 3.8-rc regression with pps-ldisc due to 70ece7a731

[George Spelvin]

> As this is a breakage in 3.8-rc1, is there any way to pull out patches 5
> and 6 in a format that I can apply them now, or in a way that I can get
> I can't apply all of these to 3.8-final now, as it's way too late.

I'll reorder them for you.  But Philipp will be bitterly disappointed. :-)

The other cleanups are far smaller and lower risk than the infrastructure
added in #5; I was just being OCD about committing the "least patchable
unit".

Re: [PATCH 0/7] 3.8-rc regression with pps-ldisc due to 70ece7a731

[George Spelvin]

As a followup, testing my fixes has revealed an old bug in the
PPS driver that I'm trying to figure out how to solve.

Basically, pps_unregister_cdev does

        device_destroy(pps_class, pps->dev->devt);
        cdev_del(&pps->cdev);

And device_destroy ends up calling pps->dev->release,
which is pps_device_destruct, which does

        mutex_lock(&pps_idr_lock);
        idr_remove(&pps_idr, pps->id);
        mutex_unlock(&pps_idr_lock);

        kfree(dev);
        kfree(pps);

Now the problem is that the kfree(pps) happens *before* the
cdev_del(&pps->cdev) call, which is Not Good.

I'm trying to figure out The Right Thing to do in this case and include a
fix for that, too.  It's not a regression, but it is a fairly serious bug.

Advice gratefully received, but I'll figure it out on my own, if not.
The most obvious kludge is to wrap the pps_unregister_cdev operations
in device_get/device_put, to force the release callback to be delayed
until later.

I'm hoping for something prettier, though.  The other option I'm thinking
about is to move one or both deallocations to the (currently stub)
pps_cdev_release function.