Subject: Re: [PATCH] ppp: fix segfaults introduced by netdev_priv changes

From: James Chapman <[email-masked]>
Date: Thu, 18 Dec 2008 10:24:43 +0000

Wang Chen wrote:

> I have no comment on these changes, but
> there is a path that calls only ppp_destroy_interface() no
> ppp_shutdown_interface().
> it's:
> ppp_unregister_channel
>     --->ppp_disconnect_channel()
>         ---> ppp_destroy_interface()

Isn't ppp_shutdown_interface() always called first? If not, then we need
to unregister_netdev() before the free_netdev() call in
ppp_destroy_interface(). The original code didn't cover that case.

>>  /*
>> @@ -2616,7 +2614,7 @@ ppp_connect_channel(struct channel *pch, int unit)
>>  	if (pch->file.hdrlen > ppp->file.hdrlen)
>>  		ppp->file.hdrlen = pch->file.hdrlen;
>>  	hdrlen = pch->file.hdrlen + 2;	/* for protocol bytes */
>> -	if (ppp->dev && hdrlen > ppp->dev->hard_header_len)
>> +	if (hdrlen > ppp->dev->hard_header_len)
>>  		ppp->dev->hard_header_len = hdrlen;
>>  	list_add_tail(&pch->clist, &ppp->channels);
>>  	++ppp->n_channels;
> 
> I don't understand this change.
> Do you mean that in this place, ppp->dev will never be NULL?

After my change, yes. See the comment in the patch preamble.

-- 
James Chapman
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development