lkml.org 
[lkml]   [2002]   [Jul]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectPatch to loop.c - fixes locking issues (IMHO)
Hi,

I think the time has come again, to start a discussion
about the way locking is done in drivers/block/loop.c

Assume we use an unmodified kernel and plug in an
encryption module (lets say a twofish.o module).

Then we do the following:

dd if=/dev/zero of=foo bs=1024 count=1024
losetup -e twofish /dev/loop0 foo
mke2fs /dev/loop0
rmmod twofish
mount /dev/loop0 /mnt

-> funny doesn't work...

What happened:
losetup doesn't lock "twofish.o", because the following
functions are called in the following order:

lo_open
no encryption module defined yet, so no locking is done
loop_set_status
calls loop_init_xfer, which will lock twofish.o once
lo_release
encryption module is unlocked

I think this is simply wrong, because
a) People don't expect this.
b) People probably don't want this.
c) A correct locking scheme will make the code
in loop_unregister_transfer quite useless, because
it will ensure that loop_unregister_transfer is only
called from a module that isn't used by any loop device.

My patch simply removes the locking/unlocking from lo_open
and lo_release.
(I left in the code in loop_unregister_transfer because
this makes the change in code as small as possible and
the code doesn't hurt even if I think it's obsolete.)

The result is that an encryption module is locked as many times
as there are loop devices using it.
The module will be locked as soon as "loop_set_status" is called
and it will be unlocked when "loop_clr_fd" is called.

So the number of locks on the encryption module corresponds to the
number of loop devices using the encryption module...

Please can someone help me get this patch into the kernel...
(I tried at least 3 times and have failed so far...)

so long
Ingo Rohloff
--- loop-orig.c Thu Jul 11 05:25:05 2002
+++ loop.c Sat Jul 13 03:56:14 2002
@@ -902,7 +902,7 @@
static int lo_open(struct inode *inode, struct file *file)
{
struct loop_device *lo;
- int dev, type;
+ int dev;

if (!inode)
return -EINVAL;
@@ -917,10 +917,6 @@
lo = &loop_dev[dev];
MOD_INC_USE_COUNT;
down(&lo->lo_ctl_mutex);
-
- type = lo->lo_encrypt_type;
- if (type && xfer_funcs[type] && xfer_funcs[type]->lock)
- xfer_funcs[type]->lock(lo);
lo->lo_refcnt++;
up(&lo->lo_ctl_mutex);
return 0;
@@ -929,7 +925,7 @@
static int lo_release(struct inode *inode, struct file *file)
{
struct loop_device *lo;
- int dev, type;
+ int dev;

if (!inode)
return 0;
@@ -944,11 +940,7 @@

lo = &loop_dev[dev];
down(&lo->lo_ctl_mutex);
- type = lo->lo_encrypt_type;
--lo->lo_refcnt;
- if (xfer_funcs[type] && xfer_funcs[type]->unlock)
- xfer_funcs[type]->unlock(lo);
-
up(&lo->lo_ctl_mutex);
MOD_DEC_USE_COUNT;
return 0;
\
 
 \ /
  Last update: 2005-03-22 13:27    [W:0.025 / U:0.456 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site