netdev - Re: circular locking, mirred, 2.6.24.2

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: <1204846839.4431.23.camel@localhost>
Date:	Thu, 06 Mar 2008 18:40:39 -0500
From:	jamal <hadi@...erus.ca>
To:	Jarek Poplawski <jarkao2@...il.com>
Cc:	Denys Fedoryshchenko <denys@...p.net.lb>, netdev@...r.kernel.org
Subject: Re: circular locking, mirred, 2.6.24.2

Jarek,

On Thu, 2008-06-03 at 22:40 +0100, Jarek Poplawski wrote:

> But currently lockdep doesn't know dev->queue_lock could mean eth or lo.
> It sees one class of devices using one lock. 

yikes ;-> I missed the implication when you mentioned it first - thanks
for the enlightnment. IMO, it needs to be per netdevice not global; so i
contend this is the problem i.e those warnings are false.

> We can let it know (e.g.
> dev->_xmit_lock is different according to dev->type), but it wasn't
> necessary. I hope it will suffice here if lockdep knows more about ifb,
> but similar problem could theoretically happen with other devs.

I think the condition needs to be met for all netdevices, not just ifb.
Instead of annotating each netdevice type separately, could you not use
a different annotation in the generic netdev registration code of the
form dev_%s->ingress/queue_lock ? (where %s is dev->name)

> Sorry, should be:
>  dev_eth0->queue_lock, and thread3 has dev_eth0->queue_lock and wants

indeed - i understood when you said that; it doesnt invalidate what i
said.

> > thread3 can not happen because we dont allow it (the other 2 are not
> > contentious).
> 
> Could you explain why? 

Because we never redirect to ingress (it is in the todo as mentioned as
described at the top of the source file). You can redirect from ingress
or egress but only to egress (to sockets and ingress are going to happen
at some future point).

> Yes, but it seems such redirection between two eths like above mentioned
> is legal?

Indeed it is. I think it is clear to me now the issues seem to be the
namespace of what lockdep - it should be per-device and NOT global.
What do you think?

cheers,
jamal

--
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