On (23/05/17 20:05), Sergey Senozhatsky wrote:
> On (23/05/17 09:59), HexRabbit wrote:
> > [ 3350.990282] BUG: KASAN: slab-out-of-bounds in smb2_handle_negotiate+0x35d7/0x3e60
> > [ 3350.990282] Read of size 2 at addr ffff88810ad61346 by task kworker/5:0/276
> > [ 3351.000406] Workqueue: ksmbd-io handle_ksmbd_work
> > [ 3351.003499] Call Trace:
> > [ 3351.006473] <TASK>
> > [ 3351.006473] dump_stack_lvl+0x8d/0xe0
> > [ 3351.006473] print_report+0xcc/0x620
> > [ 3351.006473] kasan_report+0x92/0xc0
> > [ 3351.006473] smb2_handle_negotiate+0x35d7/0x3e60
> > [ 3351.014760] ksmbd_smb_negotiate_common+0x7a7/0xf00
> > [ 3351.014760] handle_ksmbd_work+0x3f7/0x12d0
> > [ 3351.014760] process_one_work+0xa85/0x1780
>
> [..]
>
> > - if (req->DialectCount == 0) {
> > - pr_err("malformed packet\n");
> > + smb2_buf_len = get_rfc1002_len(work->request_buf);
> > + smb2_neg_size = offsetof(struct smb2_negotiate_req, Dialects);
> > + if (smb2_neg_size > smb2_buf_len) {
> > rsp->hdr.Status = STATUS_INVALID_PARAMETER;
> > rc = -EINVAL;
> > goto err_out;
> > }
> >
> > - smb2_buf_len = get_rfc1002_len(work->request_buf);
> > - smb2_neg_size = offsetof(struct smb2_negotiate_req, Dialects);
> > - if (smb2_neg_size > smb2_buf_len) {
> > + if (req->DialectCount == 0) {
> > + pr_err("malformed packet\n");
> > rsp->hdr.Status = STATUS_INVALID_PARAMETER;
> > rc = -EINVAL;
> > goto err_out;
>
> May I please ask where out-of-bounds access happens and how does
> `smb2_neg_size > smb2_buf_len` fix it?
Correction: I meant to ask "how does moving `smb2_neg_size > smb2_buf_len`
up fixes it?".
We have this in the code at the moment
```
if (req->DialectCount == 0) {
pr_err("malformed packet\n");
rsp->hdr.Status = STATUS_INVALID_PARAMETER;
rc = -EINVAL;
goto err_out;
}
smb2_buf_len = get_rfc1002_len(work->request_buf);
smb2_neg_size = offsetof(struct smb2_negotiate_req, Dialects);
if (smb2_neg_size > smb2_buf_len) {
rsp->hdr.Status = STATUS_INVALID_PARAMETER;
rc = -EINVAL;
goto err_out;
}
```
But if we move `smb2_neg_size > smb2_buf_len` brunch up, then it cures
out-of-bounds access? Where is that out-of-bounds access? Looking at
the stack trace, smb2_handle_negotiate+0x35d7/0x3e60 should be somewhere
much-much later than these if-s.
