lkml.org 
[lkml]   [2006]   [Apr]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH][resend] ISDN: unsafe interaction between isdn_write and isdn_writebuf_stub
On 21.04.2006 08:14, Andrew Morton wrote:
> It's simpler to code it this way:
[snip]
> But the code still looks wrong. If isdn_writebuf_stub() does a short write, we'll
> just retry the entire write. And if that returns the same short write, we'll
> retry the write again, ad infinitum.

You're right of course. But as i4l has never been able to handle that
case correctly, i4l drivers just don't do short writes, period. They
either return a negative error code, zero to indicate "not ready",
or the requested length. Every i4l driver author learns that very
quickly. (At least I did. ;-)

> One would expect that if a short write happened, we either bale out with an
> error or we advance partway through the buffer and write some more.

isdn_write() is the write method of i4l's character device. If a short
write would indeed occur, just passing it on to the caller should be
ok, too. So I'd propose the following, even simpler version:



From: Jesper Juhl <jesper.juhl@gmail.com>

isdn_writebuf_stub() forgets to detect memory allocation and uaccess errors.
And when that's fixed, if a error happens the caller will just keep on
looping.

So change the caller to detect the error, and to return it.

Signed-off-by: Jesper Juhl <jesper.juhl@gmail.com>
Cc: Karsten Keil <kkeil@suse.de>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Tilman Schmidt <tilman@imap.cc>

---

drivers/isdn/i4l/isdn_common.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)

diff -puN linux-2.6.17-rc2-orig/drivers/isdn/i4l/isdn_common.c linux-2.6.17-rc2-work/drivers/isdn/i4l/isdn_common.c
--- linux-2.6.17-rc2-orig/drivers/isdn/i4l/isdn_common.c 2006-03-20 06:53:29.000000000 +0100
+++ linux-2.6.17-rc2-work/drivers/isdn/i4l/isdn_common.c 2006-04-21 15:09:41.000000000 +0200
@@ -1177,9 +1177,8 @@ isdn_write(struct file *file, const char
goto out;
}
chidx = isdn_minor2chan(minor);
- while (isdn_writebuf_stub(drvidx, chidx, buf, count) != count)
+ while ((retval = isdn_writebuf_stub(drvidx, chidx, buf, count)) == 0)
interruptible_sleep_on(&dev->drv[drvidx]->snd_waitq[chidx]);
- retval = count;
goto out;
}
if (minor <= ISDN_MINOR_CTRLMAX) {
@@ -1951,9 +1950,10 @@ isdn_writebuf_stub(int drvidx, int chan,
struct sk_buff *skb = alloc_skb(hl + len, GFP_ATOMIC);

if (!skb)
- return 0;
+ return -ENOMEM;
skb_reserve(skb, hl);
- 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)
dev_kfree_skb(skb);
--
Tilman Schmidt E-Mail: tilman@imap.cc
Bonn, Germany
Reality is that which, when you stop believing in it, does not go
away. (Philip K. Dick)
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2006-04-21 14:44    [W:0.063 / U:0.460 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site