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: Mon, 10 Nov 2008 16:29:07 -0800 From: "Lovich, Vitali" <vlovich@...lcomm.com> To: "Lovich, Vitali" <vlovich@...lcomm.com>, Johann Baudy <johaahn@...il.com> CC: Evgeniy Polyakov <zbr@...emap.net>, "netdev@...r.kernel.org" <netdev@...r.kernel.org> Subject: RE: [PATCH] Packet socket: mmapped IO: PACKET_TX_RING > Also, there's no need to support both header versions in the tx path - > since v2 should be used anyways and there's no legacy (v1 headers are > deprecated no?), we can use v2 only in the tx path. There's also a minor typo anyways in tpacket_fill_skb where you read tp_len. You check for the header version, & then load the tp_len from tpacket_hdr always (this code should be removed anyways). The assignment of -EFAULT to err write before filling the fragments is redundant since err is never used after that. You don't reset the tp_status flag when you reject a message for being too short. You incorrectly calculate to_write in the situation of sock->type == SOCK_DGRAM. You don't check for the MAX_SKB_FRAGS condition. Not your code, but in packet_set_ring, it's redundant to clear the status for all the frames to TP_STATUS_KERNEL (0) since the pages are allocated with kzalloc (pages are already zeroed). I surrounded this with a CONFIG_DEBUG_KERNEL block. Earlier, I think I had said that the mapped variable should be placed in packet_ring_buffer struct - I was wrong. It's correct where it is. I had forgotten that rx & tx are mapped contiguously and simultaneously. There needs to be a note in the doc that the rings should be set up before mapping (i.e. cannot set up the rx or tx ring buffer first, mmap, and then try to change it). We do need still need a check for tx_pending_skb when sizing the ring buffer (in the situation where we unfreed skbs created by a call to send from Thread B, while Thread A unmaps the ring buffer and trys a resize). Also, in tpacket_snd, we need to increment mapped before allocating the skb and test if the original value was 0 (we decrement after incrementing tx_pending_skb). If it was, then that indicates that the user had unmapped the ring buffer, which means we should stop sending. No need for additional locking because of the atomic relationship this sets up between tx_pending_skb and mapped. I'll be posting my suggested patch tomorrow. -- 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