eg_30_netdev vulnerabilities

[sefnix]

Hi,
One of the vulnerabilities is a buffer overflow issue in the ldd_dev_rx() function. This function is called when a packet is received by the network device, and it copies the packet data into a newly allocated sk_buff object. However, the size of the allocated sk_buff is only the size of the packet data, which means that if the packet data is larger than the allocated buffer, it will overflow. This can potentially lead to arbitrary code execution.

Another vulnerability is the use of spin_lock_irqsave() and spin_unlock_irqrestore() without checking the return values. These functions can fail if called from an interrupt context with interrupts disabled, which can cause a kernel panic.

There is also a race condition in the ldd_enqueue_buf() function. This function adds received packets to a queue, but it does not use any synchronization mechanisms to protect against concurrent access. This can potentially result in packets being added to the queue out of order, leading to unpredictable behavior.

Finally, there is a potential deadlock in the ldd_release_buffer() function. This function releases packets back to the packet pool, and it checks whether the queue is stopped. If it is, it calls netif_wake_queue() to restart the queue. However, this function acquires a spinlock, which can potentially lead to a deadlock if another thread is already holding the lock and waiting for a packet to be released.

[d0u9]

Hi, @sazak

Thank you for your review.

1. I cannot clearly understand the vulnerability you described. I paste the code snippet below:

   skb = dev_alloc_skb(pkt->datalen + 2); // TODO: why add 2?
   skb_reserve(skb, 2); /* align IP on 16B boundary */  
   memcpy(skb_put(skb, pkt->datalen), pkt->data, pkt->datalen);

   I allocate an skb buffer with length pkt->datalen + 2, and then copy pkt->datalen bytes from the packet to this buffer. Can you be more clear on the overflow?

2. As far as I know, spin_lock_irqsave() returns nothing, it is just a macro in which some fundamental functions are wrapped. Search this macro in the kernel source tree, no examples by which the return value is checked.

3. ldd_enqueue_buf() internally uses irq_spinlock to prevent from race condition.

4. Do you mean that netif_wake_queue() can be intentionally called concurrently by multiple threads?

[Gerladwahl]

skb = dev_alloc_skb(pkt->datalen + 2); // TODO: why add 2?
skb_reserve(skb, 2); /* align IP on 16B boundary */
memcpy(skb_put(skb, pkt->datalen), pkt->data, pkt->datalen);