Re: [PATCH] ksmbd: fix slab-out-of-bounds read in smb2_handle_negotiate

[Sergey Senozhatsky <senozhatsky@chromium.org>]

On (23/05/17 19:45), Hex Rabbit wrote:
>    The out-of-bounds access is triggered by `req->DialectCount` memory
>    access,
>    sender can construct a malformed packet that only has a single 
>    `smb2_negotiate_req.StructureSize` field after the smb2 header.

Oh, I see, is that what you did in your reproducer?

>    Referring to the payload I showed below, since the function is 
>    assuming that the entire `smb2_negotiate_req` structure is presented, 
>    it will read above the `StructureSize` field (2400) and trigger KASAN. 
>    So check against `smb2_buf_len` first will ensure entire structure 
>    is inside the buffer, hope this make sense!
>    ```
>    00000000: 0000 0042 fe53 4d42 4000 0000 0000 0000  ...B.SMB@.......
>    00000010: 0000 0000 0000 0000 0000 0000 0000 0000  ................
>    00000020: 0000 0000 0000 0000 0000 0000 0000 0000  ................
>    00000030: 0000 0000 0000 0000 0000 0000 0000 0000  ................
>    00000040: 0000 0000 2400                                              
>    ....$.
>    ```
>    sorry for not providing full symbolized stack trace first

Thanks for clarifications. Maybe would be nice to have some of these
lines in the commit message.

>    Call Trace:
>    dump_stack_lvl (lib/dump_stack.c:107)
>    print_report (mm/kasan/report.c:352 mm/kasan/report.c:462)
>    kasan_report (mm/kasan/report.c:574)
>    smb2_handle_negotiate (fs/ksmbd/smb2pdu.c:1060)

I'm still puzzled by smb2_handle_negotiate+0x35d7/0x3e60 in the original
stack trace. 0x35d7/0x3e60 certainly doesn't translate to "start of the
function" to me, but what do I know :)