illumos gate

If one adds an encrypted lofi image the first time and the image
contains non zeros in the crypto header the system crashes with a null
pointer exception.

How to reproduce:¶

cd $(mktemp -d)
dd if=/dev/urandom of=key bs=32 count=1
dd if=/dev/urandom of=file bs=1M count=4
lofiadm -k key -c aes-256-cbc -a file ### Panic

Workaround:¶

dd if=/dev/urandom of=file bs=1M count=4
Zero out the crypto header.
dd if=/dev/zero of=file bs=512 seek=16 count=1 conv=notrunc
lofiadm -k key -c aes-256-cbc -a file ### Works

Analysis¶

usr/src/uts/common/io/lofi.c
usr/src/common/list/list.c

In lofi_map_file() from line lofi.c:2838 onward lofi_destroy() is called
on error. But between this point and the call to
list_insert_tail(&lofi_list, lsp) (lofi.c:2913) there are several goto
err calls. Since one of the first things lofi_destroy() does is calling
list_remove(&lofi_list, lsp) (lofi.c:537) we end up calling list remove
with an not yet added object.

list_remove() (list.c:131) would have failed the ASSERT(lold->list_next
!= NULL) (list.c:135) on a debug build but tries to access
lold->list_prev->list_next in list_remove_node() (list.c:65) on a
non-debug build. lold->list_prev is NULL so we crash with an null
pointer dereference.

Fix¶

Move the call to list_insert_tail(&lofi_list, lsp) near the initialization
of lsp. I'll prepare a webrev, but here is a quick patch:

diff --git a/usr/src/uts/common/io/lofi.c b/usr/src/uts/common/io/lofi.c
index b2af209..568242e 100644
--- a/usr/src/uts/common/io/lofi.c
+++ b/usr/src/uts/common/io/lofi.c
@@ -2838,6 +2838,7 @@ lofi_map_file(dev_t dev, struct lofi_ioctl *ulip, int pickminor,
         * from this point lofi_destroy() is used to clean up on error
         * make sure the basic data is set
         */
+       list_insert_tail(&lofi_list, lsp);
        lsp->ls_dev = makedevice(getmajor(dev), LOFI_ID2MINOR(id));

        list_create(&lsp->ls_comp_cache, sizeof (struct lofi_comp_cache),
@@ -2910,7 +2911,6 @@ lofi_map_file(dev_t dev, struct lofi_ioctl *ulip, int pickminor,
                }
        }

-       list_insert_tail(&lofi_list, lsp);
        /*
         * Notify we are ready to rock.
         */

Details

> ::status
debugging crash dump vmcore.1 (64-bit) from 08-00-27-b5-ae-ac
operating system: 5.11 joyent_20170119T014200Z (i86pc)
image uuid: (not set)
panic message:
BAD TRAP: type=e (#pf Page fault) rp=ffffff0008ba42e0 addr=0 occurred in module "genunix" due to a NULL pointer dereference
dump content: kernel pages only

> ::stack
list_remove+0x1b(ffffffffc00d0b00, ffffff0257345ac0)
lofi_destroy+0x60(ffffff0257345ac0, ffffff025a3b7640)
lofi_map_file+0x3de(2400000000, 8046f70, 0, ffffff0008ba4e58, ffffff025a3b7640, 100403)
lofi_ioctl+0x1d0(2400000000, 4c4602, 8046f70, 100403, ffffff025a3b7640, ffffff0008ba4e58)
cdev_ioctl+0x39(2400000000, 4c4602, 8046f70, 100403, ffffff025a3b7640, ffffff0008ba4e58)
spec_ioctl+0x60(ffffff0259cd3800, 4c4602, 8046f70, 100403, ffffff025a3b7640, ffffff0008ba4e58)
fop_ioctl+0x55(ffffff0259cd3800, 4c4602, 8046f70, 100403, ffffff025a3b7640, ffffff0008ba4e58)
ioctl+0x9b(3, 4c4602, 8046f70)
_sys_sysenter_post_swapgs+0x153()

> list_remove::dis
list_remove:                    pushq  %rbp
list_remove+1:                  movq   %rsp,%rbp
list_remove+4:                  subq   $0x10,%rsp
list_remove+8:                  movq   %rsi,-0x10(%rbp)
list_remove+0xc:                addq   0x8(%rdi),%rsi
list_remove+0x10:               movq   %rdi,-0x8(%rbp)
list_remove+0x14:               movq   0x8(%rsi),%rax
list_remove+0x18:               movq   (%rsi),%rdx

list_remove+0x1b:               movq   %rdx,(%rax)
list_remove+0x1e:               movq   (%rsi),%rax
list_remove+0x21:               movq   0x8(%rsi),%rdx
list_remove+0x25:               movq   %rdx,0x8(%rax)
list_remove+0x29:               movq   $0x0,0x8(%rsi)
list_remove+0x31:               movq   $0x0,(%rsi)
list_remove+0x38:               addq   $0x10,%rsp
list_remove+0x3c:               leave
list_remove+0x3d:               ret

> ::regs
%rax = 0x0000000000000000
%rbx = 0xffffff0257345ac0
%rcx = 0x0000000000000001
%rdx = 0x0000000000000000
%rsi = 0xffffff0257345c90
%rdi = 0xffffffffc00d0b00       lofi_list
%rip = 0xfffffffffba4143b list_remove+0x1b

Mapping to C source:

%rdi:    lofi_list
%rsi:    lold
(%rsi):  lold->list_next
8(%rsi): lold->list_prev
%rdx:    lold->list_next
(%rax):  lold->list_prev->list_next

> ffffff0257345c90::dump -q
                   \/ 1 2 3  4 5 6 7  8 9 a b  c d e f
fffff0257345c90:  00000000 00000000 00000000 00000000