KSPP / linux

Linux kernel source tree (Kernel Self Protection Project)
https://kernsec.org/wiki/index.php/Kernel_Self_Protection_Project
Other
81 stars 5 forks source link

Find and squash common netlink access mispattern #169

Open kees opened 2 years ago

kees commented 2 years ago

There have been bugs of the form:

size_t size = msg_data_sz(header);
u8 *data = msg_data(header);

struct something *instance = data + offset;

This needs to use a common accessor (like skb kind of uses), so that both offset and sizeof(*instance) are checked against overflows vs size. (Note that offset may be 0 or not added at all, and instance may be a pointer to a non-struct type.) For example, something like this would be able to do all the overflow checking:

struct something *instance;
int rc = msg_data_extract(&instance, header, offset);
if (rc)
    return rc;

See also issue #140

kees commented 1 year ago

@kuba-moo

kuba-moo commented 1 year ago

We don't have an API for just length checking currently. We have two APIs, AFAIK: pskb_may_pull() and skb_header_pointer(). The difference between the two is that the first one pulls the data into the linear portion of the skb while the latter may return a pointer to a scratch buffer if whatever it wants to access is in paged memory. Note that IIRC neither of them guarantees that caller is granted write access, appropriate CoW helper needs to be called before writing. Your helper suggestion seems good (modulo the fact that my eyes need to get used to the pointer-type-implies-size concept), but you'd need to answer the two aspects from above - does the helper pull or fail? Does it CoW or returns read-only pointer? Or perhaps it can only be used on non-cloned skbs?