netdev - Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

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: <20080221085959.GA12944@ff.dom.local>
Date:	Thu, 21 Feb 2008 08:59:59 +0000
From:	Jarek Poplawski <jarkao2@...il.com>
To:	James Chapman <jchapman@...alix.com>
Cc:	David Miller <davem@...emloft.net>,
	Paul Mackerras <paulus@...ba.org>, netdev@...r.kernel.org
Subject: Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

On Wed, Feb 20, 2008 at 10:37:57PM +0000, James Chapman wrote:
> Jarek Poplawski wrote:
>
>>>> (testing patch #1)
>>
>> But I hope you tested with the fixed (take 2) version of this patch...
>
> Yes I did. :)
>
> But I just got another lockdep error (attached).
>
>> Since it's quite experimental (testing) this patch could be wrong
>> as it is, but I hope it should show the proper way to solve this
>> problem. Probably you did some of these, but here are a few of my
>> suggestions for testing this:
>>
>> 1) try my patch with your full bh locking changing patch;
>> 2) add while loops to these trylocks on failure, with e.g.  __delay(1);
>>    this should work like full locks again, but there should be no (this
>>    kind of) lockdep reports;
>
> Hmm, isn't this just bypassing the lockdep checks?

Yes! But it's only for debugging: to find if this change in locking
is to be blamed for these new lockups. It should effectively work just
like without this patch, but without this lockdep warning. So, if
after such change lockups still happen, then it would seem you didn't
test this enough before. Otherwise the new patch is to blame and needs
reworking.

>
>> 3) I send here another testing patch with this second way to do this:
>>    on the write side, but it's even more "experimental" and only a
>>    proof of concept (should be applied on vanilla ppp_generic).
>
> I'll look over it. I think I need to take a step back and look at what's  
> happening in more detail though.

This is something completely new and changes all the picture: the xmit
path wasn't expected (at least by me) to be called in softirq context
at all, and there were no traces of this on previous reports. But,
since lockdep always stops after the first warning, there could be
even more surprises like this in the future. I'll check this report.

Regards,
Jarek P.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html