On Sun, 2001-12-02 at 23:46, Andrea Arcangeli wrote:
> > >> ps: any chance to get a sector-based-IV calculation (instead of the
> > >> actual broken soft blocksize based one) into loop.c?!?
> > > I can extract all loop.c bug fixes from loop-AES, excluding AES cipher, if
> > > someone wants them. Well, I can include AES cipher too, but that would
> > > royally piss-off the cryptoapi people.
> > ..maybe :-))
> >
> > > Does anyone want the bug fixes? Jens? Marcelo?
> > I hope jens & andrea still remember the motivation this IV thing... :-)
> Of course I remeber. I still vote for breaking the IV API and to avoid
> the compatibility cruft. Please post to l-k the patch to change the IV
> granularity from the softblocksize to 512 fixed describing our
> discussion, so if anybody really cares about the current IV API he will
> have a chance to complain before we post the patch for inclusion to
> Marcelo and Linus.
> > btw, I don't care, whether my backward-compatible (or
> > 'toothpaste-back-into-tube'-approach as jari
> > would call it ;) patch gets approved or whether a radical switch to sector
> > based IV calculation as jari proposes gets accepted...
> >
> > we just need a consistent IV metric, regardless of the underlying medium
> > (/dev/cdrom,/dev/fd0,/dev/hda,...) or any involved layers (lvm, md, ...)
>
> Indeed.
well, I've put one patch together (it still needs (constructive)
auditing though! jari?) here it is (it's against 2.4.16's loop.[ch])
(also available as /pub/linux/kernel/people/hvr/loop2-iv-2.4.16.patch)
Index: drivers/block/loop.c
===================================================================
RCS file: /cvs/linux-2.4-xfs/linux/drivers/block/loop.c,v
retrieving revision 1.43
diff -u -r1.43 loop.c
--- drivers/block/loop.c 2001/11/20 18:59:02 1.43
+++ drivers/block/loop.c 2001/12/03 15:03:36
@@ -85,7 +85,7 @@
* Transfer functions
*/
static int transfer_none(struct loop_device *lo, int cmd, char *raw_buf,
- char *loop_buf, int size, int real_block)
+ char *loop_buf, int size, loop_iv_t IV)
{
if (raw_buf != loop_buf) {
if (cmd == READ)
@@ -98,7 +98,7 @@
}
static int transfer_xor(struct loop_device *lo, int cmd, char *raw_buf,
- char *loop_buf, int size, int real_block)
+ char *loop_buf, int size, loop_iv_t IV)
{
char *in, *out, *key;
int i, keysize;
@@ -186,7 +186,7 @@
len = bh->b_size;
data = bh->b_data;
while (len > 0) {
- int IV = index * (PAGE_CACHE_SIZE/bsize) + offset/bsize;
+ const loop_iv_t IV = (index << (PAGE_CACHE_SHIFT - LOOP_IV_SECTOR_BITS)) + (offset >> LOOP_IV_SECTOR_BITS);
int transfer_result;
size = PAGE_CACHE_SIZE - offset;
@@ -244,7 +244,7 @@
unsigned long count = desc->count;
struct lo_read_data *p = (struct lo_read_data*)desc->buf;
struct loop_device *lo = p->lo;
- int IV = page->index * (PAGE_CACHE_SIZE/p->bsize) + offset/p->bsize;
+ const loop_iv_t IV = (page->index << (PAGE_CACHE_SHIFT - LOOP_IV_SECTOR_BITS)) + (offset >> LOOP_IV_SECTOR_BITS);
if (size > count)
size = count;
@@ -296,20 +296,6 @@
return bs;
}
-static inline unsigned long loop_get_iv(struct loop_device *lo,
- unsigned long sector)
-{
- int bs = loop_get_bs(lo);
- unsigned long offset, IV;
-
- IV = sector / (bs >> 9) + lo->lo_offset / bs;
- offset = ((sector % (bs >> 9)) << 9) + lo->lo_offset % bs;
- if (offset >= bs)
- IV++;
-
- return IV;
-}
-
static int do_bh_filebacked(struct loop_device *lo, struct buffer_head *bh, int rw)
{
loff_t pos;
@@ -455,7 +441,7 @@
{
struct buffer_head *bh = NULL;
struct loop_device *lo;
- unsigned long IV;
+ loop_iv_t IV;
if (!buffer_locked(rbh))
BUG();
@@ -502,7 +488,7 @@
* piggy old buffer on original, and submit for I/O
*/
bh = loop_get_buffer(lo, rbh);
- IV = loop_get_iv(lo, rbh->b_rsector);
+ IV = rbh->b_rsector + (lo->lo_offset >> LOOP_IV_SECTOR_BITS);
if (rw == WRITE) {
set_bit(BH_Dirty, &bh->b_state);
if (lo_do_transfer(lo, WRITE, bh->b_data, rbh->b_data,
@@ -539,7 +525,7 @@
bh->b_end_io(bh, !ret);
} else {
struct buffer_head *rbh = bh->b_private;
- unsigned long IV = loop_get_iv(lo, rbh->b_rsector);
+ const loop_iv_t IV = rbh->b_rsector + (lo->lo_offset >> LOOP_IV_SECTOR_BITS);
ret = lo_do_transfer(lo, READ, bh->b_data, rbh->b_data,
bh->b_size, IV);
Index: include/linux/loop.h
===================================================================
RCS file: /cvs/linux-2.4-xfs/linux/include/linux/loop.h,v
retrieving revision 1.5
diff -u -r1.5 loop.h
--- include/linux/loop.h 2001/09/21 16:28:50 1.5
+++ include/linux/loop.h 2001/12/03 15:03:36
@@ -17,6 +17,12 @@
#ifdef __KERNEL__
+/* definitions for IV metric */
+#define LOOP_IV_SECTOR_BITS 9
+#define LOOP_IV_SECTOR_SIZE (1 << LO_IV_SECTOR_BITS)
+
+typedef unsigned long loop_iv_t;
+
/* Possible states of device */
enum {
Lo_unbound,
@@ -24,6 +30,12 @@
Lo_rundown,
};
+struct loop_device;
+
+typedef int (* transfer_proc_t)(struct loop_device *, int cmd,
+ char *raw_buf, char *loop_buf, int size,
+ loop_iv_t IV);
+
struct loop_device {
int lo_number;
int lo_refcnt;
@@ -32,9 +44,7 @@
int lo_encrypt_type;
int lo_encrypt_key_size;
int lo_flags;
- int (*transfer)(struct loop_device *, int cmd,
- char *raw_buf, char *loop_buf, int size,
- int real_block);
+ transfer_proc_t transfer;
char lo_name[LO_NAME_SIZE];
char lo_encrypt_key[LO_KEY_SIZE];
__u32 lo_init[2];
@@ -58,17 +68,13 @@
atomic_t lo_pending;
};
-typedef int (* transfer_proc_t)(struct loop_device *, int cmd,
- char *raw_buf, char *loop_buf, int size,
- int real_block);
-
static inline int lo_do_transfer(struct loop_device *lo, int cmd, char *rbuf,
- char *lbuf, int size, int rblock)
+ char *lbuf, int size, loop_iv_t IV)
{
if (!lo->transfer)
return 0;
- return lo->transfer(lo, cmd, rbuf, lbuf, size, rblock);
+ return lo->transfer(lo, cmd, rbuf, lbuf, size, IV);
}
#endif /* __KERNEL__ */
@@ -122,6 +128,8 @@
#define LO_CRYPT_IDEA 6
#define LO_CRYPT_DUMMY 9
#define LO_CRYPT_SKIPJACK 10
+#define LO_CRYPT_AES 16 /* loop-AES */
+#define LO_CRYPT_CRYPTOAPI 18 /* international crypto patch */
#define MAX_LO_CRYPT 20
#ifdef __KERNEL__
@@ -129,7 +137,7 @@
struct loop_func_table {
int number; /* filter type */
int (*transfer)(struct loop_device *lo, int cmd, char *raw_buf,
- char *loop_buf, int size, int real_block);
+ char *loop_buf, int size, loop_iv_t IV);
int (*init)(struct loop_device *, struct loop_info *);
/* release is called from loop_unregister_transfer or clr_fd */
int (*release)(struct loop_device *);
--
Herbert Valerio Riedel / Phone: (EUROPE) +43-1-58801-18840
Email: [email protected] / Finger [email protected] for GnuPG Public Key
GnuPG Key Fingerprint: 7BB9 2D6C D485 CE64 4748 5F65 4981 E064 883F
4142
Herbert Valerio Riedel wrote:
> well, I've put one patch together (it still needs (constructive)
> auditing though! jari?) here it is (it's against 2.4.16's loop.[ch])
I have attached my version of loop.c bug fixes. These are extracted from
loop-AES and are well tested.
- IV computed in 512 byte units.
- Make device backed loop work with swap by pre-allocating pages.
- External encryption module locking bug fixed (from Ingo Rohloff).
- Get rid of the loop_get_bs() crap.
- grab_cache_page() return value handled properly, avoids Oops.
- No more illegal messing with BH_Dirty flag.
- No more illegal sleeping in generic_make_request().
- Loops can be set-up properly when root partition is still mounted ro.
- Default soft block size is set properly for file backed loops.
- kmalloc() error case handled properly.
Regards,
Jari Ruusu <[email protected]>
Herbert Valerio Riedel wrote:
> well, I've put one patch together (it still needs (constructive)
> auditing though! jari?) here it is (it's against 2.4.16's loop.[ch])
1) For 2.4 kernels, IV type must remain int, not loop_iv_t, ok?
Make the type loop_iv_t for 2.5 kernels but not for 2.4.
2) Get rid of the loop_get_bs() crap.
Regards,
Jari Ruusu <[email protected]>
hello!
On Mon, 2001-12-03 at 20:00, Jari Ruusu wrote:
> Herbert Valerio Riedel wrote:
>> well, I've put one patch together (it still needs (constructive)
>> auditing though! jari?) here it is (it's against 2.4.16's loop.[ch])
> 1) For 2.4 kernels, IV type must remain int, not loop_iv_t, ok?
> Make the type loop_iv_t for 2.5 kernels but not for 2.4.
no problem, the typedef can be changed to
typedef int loop_iv_t;
that way the API does not change; it just looks more consistent...
IMHO having a typedef in loop.h instead of hardcoding it to an 'int' is
more flexible and less error-prone...
> 2) Get rid of the loop_get_bs() crap.
btw, what's the motivation for it? I'd also like to know why it was used
in the first place at all... :-)
On Mon, 2001-12-03 at 23:22, Jari Ruusu wrote:
> I have attached my version of loop.c bug fixes. These are extracted from
> loop-AES and are well tested.
although I'm quite confident you patch is well tested, it goes well
beyond fixing only the IV calculation and as such represents a major
change to the loop.c driver -- I'm curious whether it will be accepted
for 2.4.x... :-) ; but don't get me wrong, I'll be quite happy if it
goes in nevertheless!
just a minor note regarding source 'aesthetics';
it would be more self-explaining IMNSHO if you used some macro constant
for the '9' shifting instead of hardcoding it...:
int IV = index * (PAGE_CACHE_SIZE >> 9) + (offset >> 9);
in your patch vs. in my patch:
/* loop.h */
#define LOOP_IV_SECTOR_BITS 9
#define LOOP_IV_SECTOR_SIZE (1 << LOOP_IV_SECTOR_BITS)
typedef int loop_iv_t;
/* loop.c */
const loop_iv_t IV =
(index << (PAGE_CACHE_SHIFT - LOOP_IV_SECTOR_BITS))
+ (offset >> LOOP_IV_SECTOR_BITS);
it makes also life easier for filters which include loop.h, cause you
can check for the presence of the #defines above, whether a fixed loop.c
is available or not; and we are also prepared for the (even if unlikely)
event that those constants are changed some time...
> - IV computed in 512 byte units.
> - Make device backed loop work with swap by pre-allocating pages.
btw, just as a side note; IMHO this is a neat feature, but encrypted
swap shouldn't be done on top of a loop device but more like the bsd
people do it, as it allows for more interesting IV & key management
schemes...
> - External encryption module locking bug fixed (from Ingo Rohloff).
> - Get rid of the loop_get_bs() crap.
> - grab_cache_page() return value handled properly, avoids Oops.
> - No more illegal messing with BH_Dirty flag.
> - No more illegal sleeping in generic_make_request().
> - Loops can be set-up properly when root partition is still mounted ro.
> - Default soft block size is set properly for file backed loops.
> - kmalloc() error case handled properly.
regards,
--
Herbert Valerio Riedel / Phone: (EUROPE) +43-1-58801-18840
Email: [email protected] / Finger [email protected] for GnuPG Public Key
GnuPG Key Fingerprint: 7BB9 2D6C D485 CE64 4748 5F65 4981 E064 883F
4142