diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 60f360b..ac21037 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -7,3 +7,7 @@ **Vulnerability:** The NATA control device (`/dev/nata_ctl`) allowed any unprivileged user to bind or unbind the virtual network interface to physical block devices via ioctl commands, leading to potential unauthorized device access and manipulation. **Learning:** Linux kernel ioctls require explicit capability checks (e.g., `capable(CAP_NET_ADMIN)`) for sensitive operations to enforce access control, even if the device file permissions are restrictive. The capability model enforces privileged operation correctly, not just file permissions. **Prevention:** Implement capability checks in device control paths (ioctls) that mutate system state or grant access to sensitive resources, such as network device bindings. +## 2024-10-24 - Out of bounds read on fragmented skb in TX path +**Vulnerability:** The NATA driver used `memcpy` to read from an `sk_buff` payload buffer. Because `sk_buff` structs can be non-linear (fragmented data spread across multiple pages), `memcpy` could trigger an out-of-bounds read and kernel panic if given a fragmented `skb`. +**Learning:** Network packet structures like `sk_buff` should never be copied directly with `memcpy` from `skb->data` for the full `skb->len` unless they are guaranteed linear. +**Prevention:** Always use `skb_copy_bits` instead of `memcpy` to read data from potentially fragmented `sk_buff` instances. diff --git a/module/nata_blk.c b/module/nata_blk.c index 66d7087..7e651ac 100644 --- a/module/nata_blk.c +++ b/module/nata_blk.c @@ -165,7 +165,8 @@ int sim_tx_packet(struct nata_priv *priv, struct sk_buff *skb, int is_dev0) /* Ensure clean valid before filling (stale clear) */ nata_slot_write_valid(slot, 0); - memcpy(slot + NATA_SLOT_PAYLOAD_OFF, skb->data, skb->len); + if (skb_copy_bits(skb, 0, slot + NATA_SLOT_PAYLOAD_OFF, skb->len)) + return -EINVAL; /* Out of bounds read on fragmented skb */ memcpy(slot + NATA_SLOT_HDR_OFF, &hdr, sizeof(hdr)); smp_wmb(); nata_slot_write_valid(slot, 1);