Skip to content

add lkl_netdev_args to consolidate signature of lkl_netdev_add#190

Merged
liuyuan10 merged 2 commits intolkl:masterfrom
liuyuan10:netdev
Aug 4, 2016
Merged

add lkl_netdev_args to consolidate signature of lkl_netdev_add#190
liuyuan10 merged 2 commits intolkl:masterfrom
liuyuan10:netdev

Conversation

@liuyuan10
Copy link
Member

@liuyuan10 liuyuan10 commented Aug 3, 2016

This change is Reviewable

Signed-off-by: Yuan Liu <liuyuan@google.com>
@hkchu
Copy link

hkchu commented Aug 3, 2016

What's the purpose for this change? (Note that "offload" is meant only to program additional features. The complete feature set is kept and available from the kernel already.)


Review status: 0 of 10 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@liuyuan10 liuyuan10 changed the title Move offload flag into lkl_netdev add lkl_netdev_args to consolidate signature of lkl_netdev_add Aug 3, 2016
@liuyuan10
Copy link
Member Author

lkl_netdev_add is heavily used by users of LKL so changing the signature is painful. I made a new change that consolidates all arguments into a struct so future args won't break existing application. PTAL


Review status: 0 of 8 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@hkchu
Copy link

hkchu commented Aug 3, 2016

:lgtm:


Review status: 0 of 8 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@ghost
Copy link

ghost commented Aug 3, 2016

Reviewed 3 of 10 files at r1, 3 of 5 files at r2.
Review status: 6 of 8 files reviewed at latest revision, 2 unresolved discussions.


tools/lkl/include/lkl.h, line 255 [r2] (raw file):

struct lkl_netdev_args {
  void *mac;
  unsigned int offload;

Small coding style issue, space at the beginning of the line.


tools/lkl/lib/hijack/init.c, line 302 [r2] (raw file):

          "LKL_HIJACK_NET_IFPARAMS instead.\n");
      nd = lkl_netdev_tap_create(tap, offload);
      if (nd) nd_args.offload = offload;

Small coding style issue, initialization should be on next line.


Comments from Reviewable

@liuyuan10
Copy link
Member Author

Review status: 6 of 8 files reviewed at latest revision, 2 unresolved discussions.


tools/lkl/include/lkl.h, line 255 [r2] (raw file):

Previously, opurdila (Octavian Purdila) wrote…

Small coding style issue, space at the beginning of the line.

Done.

tools/lkl/lib/hijack/init.c, line 302 [r2] (raw file):

Previously, opurdila (Octavian Purdila) wrote…

Small coding style issue, initialization should be on next line.

Done.

Comments from Reviewable

@ghost
Copy link

ghost commented Aug 3, 2016

:lgtm:


Reviewed 2 of 10 files at r1, 2 of 2 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@ghost
Copy link

ghost commented Aug 3, 2016

@thehajime could you also please take a look?

@thehajime
Copy link
Member

:lgtm:

Only a single minor issue.

Reviewed 5 of 10 files at r1, 1 of 5 files at r2, 2 of 2 files at r3.
Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


tools/lkl/include/lkl.h, line 256 [r3] (raw file):

  void *mac;
  unsigned int offload;
 };

There is still a space at the beginning.


Comments from Reviewable

lkl_netdev_add is heavily used by users of LKL so changing the signature
is painful. Consolidates all arguments into a struct so future args
won't break existing application.

Signed-off-by: Yuan Liu <liuyuan@google.com>
@liuyuan10
Copy link
Member Author

Review status: all files reviewed at latest revision, 1 unresolved discussion.


tools/lkl/include/lkl.h, line 256 [r3] (raw file):

Previously, thehajime (Hajime Tazaki) wrote…

There is still a space at the beginning.

Oops. Thanks for catching it.

Comments from Reviewable

@liuyuan10 liuyuan10 merged commit 0a9e7c4 into lkl:master Aug 4, 2016
@liuyuan10 liuyuan10 deleted the netdev branch August 4, 2016 01:13
thehajime pushed a commit to libos-nuse/lkl-linux that referenced this pull request Oct 17, 2016
I hit this with syzkaller:

    kasan: CONFIG_KASAN_INLINE enabled
    kasan: GPF could be caused by NULL-ptr deref or user memory access
    general protection fault: 0000 [#1] PREEMPT SMP KASAN
    CPU: 0 PID: 1327 Comm: a.out Not tainted 4.8.0-rc2+ lkl#190
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
    task: ffff88011278d600 task.stack: ffff8801120c0000
    RIP: 0010:[<ffffffff82c8ba07>]  [<ffffffff82c8ba07>] snd_hrtimer_start+0x77/0x100
    RSP: 0018:ffff8801120c7a60  EFLAGS: 00010006
    RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000007
    RDX: 0000000000000009 RSI: 1ffff10023483091 RDI: 0000000000000048
    RBP: ffff8801120c7a78 R08: ffff88011a5cf768 R09: ffff88011a5ba790
    R10: 0000000000000002 R11: ffffed00234b9ef1 R12: ffff880114843980
    R13: ffffffff84213c00 R14: ffff880114843ab0 R15: 0000000000000286
    FS:  00007f72958f3700(0000) GS:ffff88011aa00000(0000) knlGS:0000000000000000
    CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    CR2: 0000000000603001 CR3: 00000001126ab000 CR4: 00000000000006f0
    Stack:
     ffff880114843980 ffff880111eb2dc0 ffff880114843a34 ffff8801120c7ad0
     ffffffff82c81ab1 0000000000000000 ffffffff842138e0 0000000100000000
     ffff880111eb2dd0 ffff880111eb2dc0 0000000000000001 ffff880111eb2dc0
    Call Trace:
     [<ffffffff82c81ab1>] snd_timer_start1+0x331/0x670
     [<ffffffff82c85bfd>] snd_timer_start+0x5d/0xa0
     [<ffffffff82c8795e>] snd_timer_user_ioctl+0x88e/0x2830
     [<ffffffff8159f3a0>] ? __follow_pte.isra.49+0x430/0x430
     [<ffffffff82c870d0>] ? snd_timer_pause+0x80/0x80
     [<ffffffff815a26fa>] ? do_wp_page+0x3aa/0x1c90
     [<ffffffff8132762f>] ? put_prev_entity+0x108f/0x21a0
     [<ffffffff82c870d0>] ? snd_timer_pause+0x80/0x80
     [<ffffffff816b0733>] do_vfs_ioctl+0x193/0x1050
     [<ffffffff813510af>] ? cpuacct_account_field+0x12f/0x1a0
     [<ffffffff816b05a0>] ? ioctl_preallocate+0x200/0x200
     [<ffffffff81002f2f>] ? syscall_trace_enter+0x3cf/0xdb0
     [<ffffffff815045ba>] ? __context_tracking_exit.part.4+0x9a/0x1e0
     [<ffffffff81002b60>] ? exit_to_usermode_loop+0x190/0x190
     [<ffffffff82001a97>] ? check_preemption_disabled+0x37/0x1e0
     [<ffffffff81d93889>] ? security_file_ioctl+0x89/0xb0
     [<ffffffff816b167f>] SyS_ioctl+0x8f/0xc0
     [<ffffffff816b15f0>] ? do_vfs_ioctl+0x1050/0x1050
     [<ffffffff81005524>] do_syscall_64+0x1c4/0x4e0
     [<ffffffff83c32b2a>] entry_SYSCALL64_slow_path+0x25/0x25
    Code: c7 c7 c4 b9 c8 82 48 89 d9 4c 89 ee e8 63 88 7f fe e8 7e 46 7b fe 48 8d 7b 48 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <0f> b6 04 02 84 c0 74 04 84 c0 7e 65 80 7b 48 00 74 0e e8 52 46
    RIP  [<ffffffff82c8ba07>] snd_hrtimer_start+0x77/0x100
     RSP <ffff8801120c7a60>
    ---[ end trace 5955b08db7f2b029 ]---

This can happen if snd_hrtimer_open() fails to allocate memory and
returns an error, which is currently not checked by snd_timer_open():

    ioctl(SNDRV_TIMER_IOCTL_SELECT)
     - snd_timer_user_tselect()
	- snd_timer_close()
	   - snd_hrtimer_close()
	      - (struct snd_timer *) t->private_data = NULL
        - snd_timer_open()
           - snd_hrtimer_open()
              - kzalloc() fails; t->private_data is still NULL

    ioctl(SNDRV_TIMER_IOCTL_START)
     - snd_timer_user_start()
	- snd_timer_start()
	   - snd_timer_start1()
	      - snd_hrtimer_start()
		- t->private_data == NULL // boom

Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants