2.6.13: loop ioctl crashes

2.6.13: loop ioctl crashes

[Ian Collier]

I'm trying out PPDD from https://retiisi.dyndns.org/~sailus/ppdd/
because I have some old stuff in that format.  However, the crash
seems to occur in code that isn't touched by the PPDD patch.  It
happens while I'm trying to set up the loop device - I haven't got
as far as actually using it yet.

If I'm lucky then when I issue the losetup command and successfully
type in the passphrase then losetup says something of the form
ioctl: LOOP_SET_STATUS: Bad address
and the kernel says:

 Debug: sleeping function called from invalid context at arch/i386/lib/usercopy.c:634
 in_atomic():1, irqs_disabled():0
  [<c011f8eb>] __might_sleep+0xab/0xc0
  [<c0211aa3>] copy_from_user+0x23/0x90
  [<d0a9fe80>] loop_set_status_old+0x30/0x70 [loop]

However, it often seems to panic in a variety of horrible ways while
trying to print the above message.

Clearly I have CONFIG_DEBUG_SPINLOCK_SLEEP set (as my config is
based on Fedora's), and I suppose I could just try unsetting it to
make the message go away.  That wouldn't make the underlying bug go
away, though.  If it makes any difference, loop and all the crypto
algorithms are compiled as modules.

I don't understand why it's an invalid context.  I also don't understand
why the traceback stops at loop_set_status_old given that it must have
been called from lo_ioctl.  (But maybe the answer to the latter would
explain the former.)

I may try just moving the copy_from_user() out to the beginning of
lo_ioctl and see what happens.  Any other suggestions?  In case it's
not obvious by now, I'm not really a kernel hacker.

imc

Re: 2.6.13: loop ioctl crashes

[linux-os (Dick Johnson)]

On Fri, 9 Sep 2005, Ian Collier wrote:

> I'm trying out PPDD from https://retiisi.dyndns.org/~sailus/ppdd/
> because I have some old stuff in that format.  However, the crash
> seems to occur in code that isn't touched by the PPDD patch.  It
> happens while I'm trying to set up the loop device - I haven't got
> as far as actually using it yet.
>
> If I'm lucky then when I issue the losetup command and successfully
> type in the passphrase then losetup says something of the form
> ioctl: LOOP_SET_STATUS: Bad address
> and the kernel says:
>
> Debug: sleeping function called from invalid context at arch/i386/lib/usercopy.c:634
> in_atomic():1, irqs_disabled():0
>  [<c011f8eb>] __might_sleep+0xab/0xc0
>  [<c0211aa3>] copy_from_user+0x23/0x90
>  [<d0a9fe80>] loop_set_status_old+0x30/0x70 [loop]
>
> However, it often seems to panic in a variety of horrible ways while
> trying to print the above message.
>
> Clearly I have CONFIG_DEBUG_SPINLOCK_SLEEP set (as my config is
> based on Fedora's), and I suppose I could just try unsetting it to
> make the message go away.  That wouldn't make the underlying bug go
> away, though.  If it makes any difference, loop and all the crypto
> algorithms are compiled as modules.
>
> I don't understand why it's an invalid context.  I also don't understand
> why the traceback stops at loop_set_status_old given that it must have
> been called from lo_ioctl.  (But maybe the answer to the latter would
> explain the former.)
>
> I may try just moving the copy_from_user() out to the beginning of
> lo_ioctl and see what happens.  Any other suggestions?  In case it's
> not obvious by now, I'm not really a kernel hacker.
>
> imc

I guess you are trying to do a copy_from_user() with a spin-lock
being held or the interrupts otherwise disabled. You can hold
a semaphore, to prevent somebody else from interfering with
you, but you cannot hold a spin-lock during copy/to/from/user().


Cheers,
Dick Johnson
Penguin : Linux version 2.6.13 on an i686 machine (5589.54 BogoMips).
Warning : 98.36% of all statistics are fiction.
.
I apologize for the following. I tried to kill it with the above dot :

****************************************************************
The information transmitted in this message is confidential and may be privileged.  Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited.  If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to DeliveryErrors@analogic.com - and destroy all copies of this information, including any attachments, without reading or disclosing them.

Thank you.

Re: 2.6.13: loop ioctl crashes

[Ian Collier]

On Fri, Sep 09, 2005 at 08:32:10AM -0400, linux-os (Dick Johnson) wrote:
> I guess you are trying to do a copy_from_user() with a spin-lock
> being held or the interrupts otherwise disabled. You can hold
> a semaphore, to prevent somebody else from interfering with
> you, but you cannot hold a spin-lock during copy/to/from/user().

Well, I didn't write the code (it's right there in drivers/block/loop.c
in 2.6.13) and I can't see where there's a spin-lock.  In fact it does
use a semaphore.

imc

Re: 2.6.13: loop ioctl crashes

[linux-os (Dick Johnson)]

On Fri, 9 Sep 2005, Ian Collier wrote:

> On Fri, Sep 09, 2005 at 08:32:10AM -0400, linux-os (Dick Johnson) wrote:
>> I guess you are trying to do a copy_from_user() with a spin-lock
>> being held or the interrupts otherwise disabled. You can hold
>> a semaphore, to prevent somebody else from interfering with
>> you, but you cannot hold a spin-lock during copy/to/from/user().
>
> Well, I didn't write the code (it's right there in drivers/block/loop.c
> in 2.6.13) and I can't see where there's a spin-lock.  In fact it does
> use a semaphore.
>
> imc
> -

Try to see if it is really the loop device or something that is
interfacing with it. Here I copy the contents of a DOS floppy
to a file, then mount the file through the loop device:

Script started on Fri 09 Sep 2005 10:17:27 AM EDT
[root@chaos driver]# cp /dev/fd0 image
[root@chaos driver]# ls -la image
-rw-r-----  1 root root 1474560 Sep  9 10:18 image
[root@chaos driver]# mount -o loop image /mnt
[root@chaos driver]# ls -la /mnt
total 894
drwxr-xr-x   2 root root   7168 Dec 31  1969 ^[[01;34m.^[[00m
drwxr-xr-x  26 root root   4096 Sep  9 08:41 ^[[01;34m..^[[00m
-rwxr-xr-x   1 root root    170 Apr 10  2003 ^[[01;32mautoexec.bat^[[00m
-rwxr-xr-x   1 root root  86413 Jul 30  2002 ^[[01;32mcommand.com^[[00m
-rwxr-xr-x   1 root root   2882 Apr  9  2003 ^[[01;32mconfig.sys^[[00m
-rwxr-xr-x   1 root root  16967 Mar 27  2003 ^[[01;32merr_lev.bat^[[00m
-rwxr-xr-x   1 root root   5874 Jan 21  2002 ^[[01;32mfdxxms.sys^[[00m
-rwxr-xr-x   1 root root   3173 Mar 27  2003 ^[[01;32mfindramd.exe^[[00m
-rwxr-xr-x   1 root root  41293 Aug  4  2002 ^[[01;32mkernel.sys^[[00m
-rwxr-xr-x   1 root root 719592 Jun 28  2004 ^[[01;32msw.exe^[[00m
-rwxr-xr-x   1 root root  25084 Sep 28  2000 ^[[01;32mtdsk.exe^[[00m
[root@chaos driver]# umount /mnt
[root@chaos driver]# exit

Script done on Fri 09 Sep 2005 10:18:55 AM EDT



This seems to work okay in 2.6.13, however I don't think it
__should__ work because in lo_ioctl(), the following
functions reference 'arg' without using copy/to/from/user() or
put/get/user():
 	loop_set_fd (vi fget()),
 	loop_change_fd (via fget()),
 	loop_get_status (via memset() and others),
 	loop_get_status_old,
 	loop_set_status64,
 	loop_get_status64,
         ... etc.

Basically, anything that uses ioctl() on the loop device may find
that they crash the system. This code is broken.

Anton Altaparmakov last 'touched' that code in Feb 2005. Maybe
he can fix the ioctl procedure to use the correct interface to
user-land????


Cheers,
Dick Johnson
Penguin : Linux version 2.6.13 on an i686 machine (5589.53 BogoMips).
Warning : 98.36% of all statistics are fiction.
.
I apologize for the following. I tried to kill it with the above dot :

****************************************************************
The information transmitted in this message is confidential and may be privileged.  Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited.  If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to DeliveryErrors@analogic.com - and destroy all copies of this information, including any attachments, without reading or disclosing them.

Thank you.

Re: 2.6.13: loop ioctl crashes

[linux-os (Dick Johnson)]

On Fri, 9 Sep 2005, Richard B. Johnson wrote:

> On Fri, 9 Sep 2005, Ian Collier wrote:
>
>> On Fri, Sep 09, 2005 at 08:32:10AM -0400, linux-os (Dick Johnson) wrote:
>>> I guess you are trying to do a copy_from_user() with a spin-lock
>>> being held or the interrupts otherwise disabled. You can hold
>>> a semaphore, to prevent somebody else from interfering with
>>> you, but you cannot hold a spin-lock during copy/to/from/user().
>>
>> Well, I didn't write the code (it's right there in drivers/block/loop.c
>> in 2.6.13) and I can't see where there's a spin-lock.  In fact it does
>> use a semaphore.
>>
>> imc
>> -
>
> Try to see if it is really the loop device or something that is
> interfacing with it. Here I copy the contents of a DOS floppy
> to a file, then mount the file through the loop device:
>
> Script started on Fri 09 Sep 2005 10:17:27 AM EDT
> [root@chaos driver]# cp /dev/fd0 image
> [root@chaos driver]# ls -la image
> -rw-r-----  1 root root 1474560 Sep  9 10:18 image
> [root@chaos driver]# mount -o loop image /mnt
> [root@chaos driver]# ls -la /mnt
> total 894
> drwxr-xr-x   2 root root   7168 Dec 31  1969 ^[[01;34m.^[[00m
> drwxr-xr-x  26 root root   4096 Sep  9 08:41 ^[[01;34m..^[[00m
> -rwxr-xr-x   1 root root    170 Apr 10  2003 ^[[01;32mautoexec.bat^[[00m
> -rwxr-xr-x   1 root root  86413 Jul 30  2002 ^[[01;32mcommand.com^[[00m
> -rwxr-xr-x   1 root root   2882 Apr  9  2003 ^[[01;32mconfig.sys^[[00m
> -rwxr-xr-x   1 root root  16967 Mar 27  2003 ^[[01;32merr_lev.bat^[[00m
> -rwxr-xr-x   1 root root   5874 Jan 21  2002 ^[[01;32mfdxxms.sys^[[00m
> -rwxr-xr-x   1 root root   3173 Mar 27  2003 ^[[01;32mfindramd.exe^[[00m
> -rwxr-xr-x   1 root root  41293 Aug  4  2002 ^[[01;32mkernel.sys^[[00m
> -rwxr-xr-x   1 root root 719592 Jun 28  2004 ^[[01;32msw.exe^[[00m
> -rwxr-xr-x   1 root root  25084 Sep 28  2000 ^[[01;32mtdsk.exe^[[00m
> [root@chaos driver]# umount /mnt
> [root@chaos driver]# exit
>
> Script done on Fri 09 Sep 2005 10:18:55 AM EDT
>
>

Ignore what I wrote following the stuff above:

The code wasn't broken after all. Function fget() didn't
dereference a pointer. It just got a number as 'arg' so
it didn't have to copy to/from anything.

The stuff that was dereferenced did use the correct
copy/to/from() code.

>
> This seems to work okay in 2.6.13, however I don't think it
> __should__ work because in lo_ioctl(), the following
> functions reference 'arg' without using copy/to/from/user() or
> put/get/user():
> 	loop_set_fd (vi fget()),
> 	loop_change_fd (via fget()),
> 	loop_get_status (via memset() and others),
> 	loop_get_status_old,
> 	loop_set_status64,
> 	loop_get_status64,
>        ... etc.
>
> Basically, anything that uses ioctl() on the loop device may find
> that they crash the system. This code is broken.
>
> Anton Altaparmakov last 'touched' that code in Feb 2005. Maybe
> he can fix the ioctl procedure to use the correct interface to
> user-land????

Cheers,
Dick Johnson
Penguin : Linux version 2.6.13 on an i686 machine (5589.53 BogoMips).
Warning : 98.36% of all statistics are fiction.
.
I apologize for the following. I tried to kill it with the above dot :

****************************************************************
The information transmitted in this message is confidential and may be privileged.  Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited.  If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to DeliveryErrors@analogic.com - and destroy all copies of this information, including any attachments, without reading or disclosing them.

Thank you.

Re: 2.6.13: loop ioctl crashes

[Ian Collier]

On Fri, Sep 09, 2005 at 10:41:29AM -0400, linux-os (Dick Johnson) wrote:
> Try to see if it is really the loop device or something that is
> interfacing with it. Here I copy the contents of a DOS floppy
> to a file, then mount the file through the loop device:

I do know how to use loop devices. ;-)

Now you mention it, "losetup" without encryption works fine.

This: losetup -e blowfish /dev/loop0 /tmp/test.img
fails with "bad address" if the blowfish module is not loaded,
and causes a kernel panic if it is (and now I have to go home
to reset my machine. :-( ).

The only thing I can see that the ppdd patch has done here is to
increase LO_KEY_SIZE from 32 to 1844; the rest should be as in
vanilla 2.6.13 (though at some point I'll recompile without
patches and see if that changes anything).

imc

2.6.13: More on drivers/block/loop.c

[Ian Collier]

Vanilla 2.6.13 doesn't crash.

However, unpack a fresh copy of 2.6.13, edit include/linux/loop.h to
change LO_KEY_SIZE from 32 to 1844, and *boom*.  [Don't ask me why
1844... that's just what PPDD wants.]

It's crashing somewhere in loop_set_status_old, probably during the
call to copy_from_user, but the crash messages aren't that helpful as
they are different each time, often seem to happen during an interrupt,
and usually contain pages of recursive calls to do_page_fault and
error_code.

The loop_set_status_old function has two local variables, each of which
is now 1812 bytes longer than it was, and I'm wondering if it's a stack
overflow problem.  How much stack is a kernel function allowed to use,
anyway?

Replacing these variables with kmalloc'd pointers seems to stop the crashes
anyway, so I'll pass that tip on to the PPDD folks.

imc

Re: 2.6.13: More on drivers/block/loop.c

[Anton Altaparmakov]

On Wed, 2005-09-14 at 13:51 +0100, Ian Collier wrote:
> Vanilla 2.6.13 doesn't crash.
> 
> However, unpack a fresh copy of 2.6.13, edit include/linux/loop.h to
> change LO_KEY_SIZE from 32 to 1844, and *boom*.  [Don't ask me why
> 1844... that's just what PPDD wants.]
> 
> It's crashing somewhere in loop_set_status_old, probably during the
> call to copy_from_user, but the crash messages aren't that helpful as
> they are different each time, often seem to happen during an interrupt,
> and usually contain pages of recursive calls to do_page_fault and
> error_code.
> 
> The loop_set_status_old function has two local variables, each of which
> is now 1812 bytes longer than it was, and I'm wondering if it's a stack
> overflow problem.  How much stack is a kernel function allowed to use,
> anyway?
> 
> Replacing these variables with kmalloc'd pointers seems to stop the crashes
> anyway, so I'll pass that tip on to the PPDD folks.

Not surprising.  The _entirety_ of the kernel, i.e. not just each
function, has either 4k or 8k of stack (depending on a .config option)
so having two local variables of 1812 bytes each is _guaranteed_ to blow
the stack.

Best regards,

        Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

loop: auto-load crypto module [PATCH]

[Ian Collier]

It seems odd that "losetup -e blowfish" will auto-load the blowfish module
if it's not already loaded but it doesn't work if the cryptoloop module
isn't loaded.  It's fairly simple to amend loop so that it requests
a crypto module when required - a sample patch appears below.  Just put
"alias loop-encrypt-18 cryptoloop" in your modprobe.conf.  This currently
fails silently if the module isn't found - I don't know whether it's
worth putting a message in there.

On an unrelated note, while looking at loop.c I noticed that loop_init
contains two calls to memset for the same block of memory (one outside
the for-loop, one inside).  It seems to me that one of these isn't
necessary.  :-)

imc

--- linux-2.6.13/drivers/block/loop.c.orig	2005-08-29 00:41:01.000000000 +0100
+++ linux-2.6.13/drivers/block/loop.c	2005-09-14 14:01:42.844009381 +0100
@@ -74,6 +74,7 @@
 #include <linux/completion.h>
 #include <linux/highmem.h>
 #include <linux/gfp.h>
+#include <linux/kmod.h>
 
 #include <asm/uaccess.h>
 
@@ -950,8 +951,12 @@
 		if (type >= MAX_LO_CRYPT)
 			return -EINVAL;
 		xfer = xfer_funcs[type];
-		if (xfer == NULL)
-			return -EINVAL;
+		if (xfer == NULL) {
+			request_module("loop-encrypt-%u",type);
+			xfer = xfer_funcs[type];
+			if (xfer == NULL)
+				return -EINVAL;
+		}
 	} else
 		xfer = NULL;