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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
