From: Meng Xu <mengxu.gat...@gmail.com>
Date: Tue, 19 Sep 2017 14:52:58 -0400
> In isdn_ppp_write(), the header (i.e., protobuf) of the buffer is fetched
> twice from userspace. The first fetch is used to peek at the protocol
> of the message and reset the huptimer if necessary; while the second
> fetch copies in the whole buffer. However, given that buf resides in
> userspace memory, a user process can race to change its memory content
> across fetches. By doing so, we can either avoid resetting the huptimer
> for any type of packets (by first setting proto to PPP_LCP and later
> change to the actual type) or force resetting the huptimer for LCP packets.
>
> This patch does a memcmp between the two fetches and abort if changes to
> the protobuf is detected across fetches.
>
> Signed-off-by: Meng Xu <mengxu.gat...@gmail.com>
Doing a memcmp() for every buffer is expensive, ugly, and not the
way we usually handle this kind of issue.
Instead, atomically copy the entire buffer, as needed.
Something like:
struct sk_buff *skb = NULL;
unsigned char protobuf[4];
unsigned char *cpy_buf;
if (lp->isdn_device >= 0 && lp->isdn_channel >= 0 &&
(dev->drv[lp->isdn_device]->flags & DRV_FLAG_RUNNING) &&
lp->dialstate == 0 &&
(lp->flags & ISDN_NET_CONNECTED)) {
/*
* we need to reserve enough space in front of
* sk_buff. old call to dev_alloc_skb only reserved
* 16 bytes, now we are looking what the driver want
*/
hl = dev->drv[lp->isdn_device]->interface->hl_hdrlen;
skb = alloc_skb(hl + count, GFP_ATOMIC);
if (!skb) {
printk(KERN_WARNING "isdn_ppp_write: out of
memory!\n");
return count;
}
skb_reserve(skb, hl);
cpy_buf = skb_put(skb, count);
} else {
cpy_buf = protobuf;
count = sizeof(protobuf);
}
if (copy_from_user(cpy_buf, buf, count)) {
kfree_skb(skb);
return -EFAULT;
}
proto = PPP_PROTOCOL(cpy_buf);
if (proto != PPP_LCP)
lp->huptimer = 0;
...
