Re: 2.6.17-rc4-mm1

From: Tilman Schmidt
Date: Wed May 17 2006 - 20:03:19 EST


On 15.05.2006 20:10, Jesper Juhl wrote:
> In the
> isdn-unsafe-interaction-between-isdn_write-and-isdn_writebuf_stub.patch
> patch, which is currently in -mm there's a bug.
>
> This bit :
> - copy_from_user(skb_put(skb, len), buf, len);
> + if (!copy_from_user(skb_put(skb, len), buf, len))
> should really be :
> - copy_from_user(skb_put(skb, len), buf, len);
> + if (copy_from_user(skb_put(skb, len), buf, len))
> Somehow a stray "!" crept in there.

Ouch. I hadn't noticed that either. Sorry about that.

Strangely, I ran a system with that patch for quite some time, actually
doing all kinds of ISDN connections without ever noticing any ill
effects. I can only conclude that the affected codebranch was never
exercised, which makes me suspect it is dead, or at least pining for the
fjords. Looking closer, I can't help noticing that the only branch
calling the affected function isdn_writebuf_stub() starts out with:

printk(KERN_WARNING "isdn_write minor %d obsolete!\n", minor);

So I guess it really doesn't make much difference one way or the other.
But it's certainly more correct that way, so:

> Signed-off-by: Jesper Juhl <jesper.juhl@xxxxxxxxx>
Acked-by: Tilman Schmidt <tilman@xxxxxxx>

> --- linux-2.6.17-rc4-mm1-orig/drivers/isdn/i4l/isdn_common.c 2006-05-15 19:43:06.000000000 +0200
> +++ linux-2.6.17-rc4-mm1/drivers/isdn/i4l/isdn_common.c 2006-05-15 19:58:26.000000000 +0200
> @@ -1952,7 +1952,7 @@ isdn_writebuf_stub(int drvidx, int chan,
> if (!skb)
> return -ENOMEM;
> skb_reserve(skb, hl);
> - if (!copy_from_user(skb_put(skb, len), buf, len))
> + if (copy_from_user(skb_put(skb, len), buf, len))
> return -EFAULT;
> ret = dev->drv[drvidx]->interface->writebuf_skb(drvidx, chan, 1, skb);
> if (ret <= 0)

--
Tilman Schmidt E-Mail: tilman@xxxxxxx
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)

Attachment: signature.asc
Description: OpenPGP digital signature