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
| ||
|
Date: Wed, 21 Oct 2009 07:14:02 +0200 From: Eric Dumazet <eric.dumazet@...il.com> To: Ben Greear <greearb@...delatech.com> CC: NetDev <netdev@...r.kernel.org>, robert@...julf.net, "David S. Miller" <davem@...emloft.net> Subject: Re: pktgen and spin_lock_bh in xmit path Ben Greear a écrit : > Eric Dumazet wrote: >> pktgen should not use "clone XXX" pkts if macvlan is used (or any >> other driver >> that ultimatly calls dev_queue_xmit() and queue packet), since skb >> queue anchor >> is shared and would be overwritten. >> After some thoughts, I believe user is in error :) > I tried to explain in my original post: The problem arises when > when the hard-start-xmit fails with NETDEV_TX_BUSY. Part of the > hard-start-xmit logic for virtual devices can call dev_queue_xmit, which > can ultimately > change the queue mapping and yet may still return NETDEV_TX_BUSY. > > pktgen would try to resend this skb next loop, and this is where it would > blow up. > > I have a patched macvlan logic and a patched dev queue xmit logic that > allows > me to return NETDEV_TX_BUSY when underlying device fails to transmit. > > It may be that my hacked macvlan is the only virtual device that could ever > return NETDEV_TX_BUSY, and if that is the case, I don't think the bug > could ever be hit in official kernel code. My opinion is that the > current pktgen code makes > too many assumptions, so unless there is a performance penalty, I still > think it should be cleaned up. But, I may be too paranoid. If a virtual device changes skb->queue_map, it must consume skb, or it breaks caller. Alternative would be to restore queue_map to its initial value in your hacked macvlan when it wants to return NETDEV_TX_BUSY status. We could add a WARN_ON(skb_get_queue_mapping(pkt_dev->skb) != queue_map); in pktgen, to catch driver errors but pktgen assumption is right IMHO @@ -3466,6 +3471,7 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev) /* fallthru */ case NETDEV_TX_LOCKED: case NETDEV_TX_BUSY: + WARN_ON(skb_get_queue_mapping(pkt_dev->skb) != queue_map); /* Retry it next time */ atomic_dec(&(pkt_dev->skb->users)); pkt_dev->last_ok = 0; -- 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
Powered by blists - more mailing lists