Re: [PATCH] loop.c: kernel_thread() retval check
- From: Willy Tarreau <w@xxxxxx>
- Date: Sun, 20 Aug 2006 09:21:48 +0200
Hi Alexander,
On Sun, Aug 20, 2006 at 03:46:29AM +0400, Solar Designer wrote:
Willy,
I propose the attached patch (extracted from 2.4.33-ow1) for inclusion
into 2.4.34-pre. (Last time I checked, 2.6 needed an equivalent fix,
but I haven't produced one yet.)
Basically, the code in drivers/block/loop.c did not check the return
value from kernel_thread(). If kernel_thread() would fail, the code
would misbehave (IIRC, the invoking process would become unkillable).
An easy way to trigger the bug was to run losetup under strace (as
root), and this is also how I tested the error path added with this
patch.
That's amazing it never got fixed before !
I still remembered this problem being discussed, and finally found
the thread :
http://lkml.org/lkml/2003/11/14/55
In fact, no code was proposed and 2.6 got fixed later, then stopped
using kernel_thread() so nearly nobody might have noticed it :
http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff_plain;h=3e88c17d404c5787afd5bd1763380317f5ccbf84;hp=22e6c1b39c648850438decd491f62d311800c7db
This change has been a part of publicly released -ow patches for 8+
months.
There are more instances of kernel_thread() calls that do not check the
return value; some of the remaining ones might need to be fixed, too.
I've read somewhere that the module loading code might be affected too.
Thanks,
Alexander
Thanks for this patch, I know this problem caught me at least one !
Cheers,
Willy
diff -urpPX nopatch linux-2.4.33/drivers/block/loop.c linux/drivers/block/loop.c
--- linux-2.4.33/drivers/block/loop.c Fri Jun 3 04:26:42 2005
+++ linux/drivers/block/loop.c Sat Aug 12 08:51:47 2006
@@ -693,12 +693,23 @@ static int loop_set_fd(struct loop_devic
set_blocksize(dev, bs);
lo->lo_bh = lo->lo_bhtail = NULL;
- kernel_thread(loop_thread, lo, CLONE_FS | CLONE_FILES | CLONE_SIGHAND);
- down(&lo->lo_sem);
+ error = kernel_thread(loop_thread, lo,
+ CLONE_FS | CLONE_FILES | CLONE_SIGHAND);
+ if (error < 0)
+ goto out_clr;
+ down(&lo->lo_sem); /* wait for the thread to start */
fput(file);
return 0;
+ out_clr:
+ lo->lo_backing_file = NULL;
+ lo->lo_device = 0;
+ lo->lo_flags = 0;
+ loop_sizes[lo->lo_number] = 0;
+ inode->i_mapping->gfp_mask = lo->old_gfp_mask;
+ lo->lo_state = Lo_unbound;
+ fput(file); /* yes, have to do it twice */
out_putf:
fput(file);
out:
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
- Follow-Ups:
- Re: [PATCH] loop.c: kernel_thread() retval check
- From: Solar Designer
- Re: [PATCH] loop.c: kernel_thread() retval check
- References:
- [PATCH] loop.c: kernel_thread() retval check
- From: Solar Designer
- [PATCH] loop.c: kernel_thread() retval check
- Prev by Date: Re: 2.6.18-rc4-mm2
- Next by Date: Re: 2.6.18-rc4-mm2
- Previous by thread: [PATCH] loop.c: kernel_thread() retval check
- Next by thread: Re: [PATCH] loop.c: kernel_thread() retval check
- Index(es):
Relevant Pages
|