Skip to content

-Wself-assign in drivers/media/pci/bt8xx/bttv-driver.c #69

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nickdesaulniers opened this issue Sep 14, 2018 · 3 comments
Closed

-Wself-assign in drivers/media/pci/bt8xx/bttv-driver.c #69

nickdesaulniers opened this issue Sep 14, 2018 · 3 comments
Assignees
Labels
-Wself-assign [BUG] linux A bug that should be fixed in the mainline kernel. [FIXED][LINUX] 4.20 This bug was fixed in Linux 4.20

Comments

@nickdesaulniers
Copy link
Member

  CC      drivers/media/pci/bt8xx/bttv-driver.o
drivers/media/pci/bt8xx/bttv-driver.c:2043:13: warning: explicitly assigning value of variable of type '__s32' (aka 'int') to itself [-Wself-assign]
        min_height = min_height;
        ~~~~~~~~~~ ^ ~~~~~~~~~~

see #68 for my thoughts on this.

@nickdesaulniers nickdesaulniers added help wanted good first issue Good for newcomers [BUG] linux A bug that should be fixed in the mainline kernel. low priority This bug is not critical and not a priority labels Sep 14, 2018
@nickdesaulniers nickdesaulniers added this to the ARCH=arm64 allyesconfig milestone Sep 14, 2018
@nathanchance
Copy link
Member

@nathanchance nathanchance added the [PATCH] Exists There is a patch that fixes this issue label Sep 15, 2018
@nathanchance nathanchance self-assigned this Sep 15, 2018
@nathanchance nathanchance added -Wself-assign [PATCH] Submitted A patch has been submitted for review and removed [PATCH] Exists There is a patch that fixes this issue labels Sep 20, 2018
@nathanchance
Copy link
Member

@nathanchance nathanchance added [PATCH] Accepted A submitted patch has been accepted upstream and removed [PATCH] Submitted A patch has been submitted for review labels Sep 24, 2018
@tpimh tpimh removed the good first issue Good for newcomers label Sep 25, 2018
@nathanchance
Copy link
Member

@nathanchance nathanchance added [FIXED][LINUX] 4.20 This bug was fixed in Linux 4.20 and removed [PATCH] Accepted A submitted patch has been accepted upstream labels Oct 30, 2018
@tpimh tpimh removed the low priority This bug is not critical and not a priority label Oct 30, 2018
nathanchance pushed a commit that referenced this issue Jan 21, 2019
signal handling core calls show_regs() with preemption disabled which
on ARC takes mmap_sem for mm/vma access, causing lockdep splat.

| [ARCLinux]# ./segv-null-ptr
| potentially unexpected fatal signal 11.
| BUG: sleeping function called from invalid context at kernel/fork.c:1011
| in_atomic(): 1, irqs_disabled(): 0, pid: 70, name: segv-null-ptr
| no locks held by segv-null-ptr/70.
| CPU: 0 PID: 70 Comm: segv-null-ptr Not tainted 4.18.0+ #69
|
| Stack Trace:
|  arc_unwind_core+0xcc/0x100
|  ___might_sleep+0x17a/0x190
|  mmput+0x16/0xb8
|  show_regs+0x52/0x310
|  get_signal+0x5ee/0x610
|  do_signal+0x2c/0x218
|  resume_user_mode_begin+0x90/0xd8

Workaround by re-enabling preemption temporarily.

Note that the preemption disabling in core code around show_regs()
was introduced by commit 3a9f84d ("signals, debug: fix BUG: using
smp_processor_id() in preemptible code in print_fatal_signal()")

to silence a differnt lockdep seen on x86 bakc in 2009.

Cc: <[email protected]>
Signed-off-by: Vineet Gupta <[email protected]>
nathanchance pushed a commit that referenced this issue Jul 13, 2019
powerpc hardware triggers watchpoint before executing the instruction.
To make trigger-after-execute behavior, kernel emulates the
instruction. If the instruction is 'load something into non-volatile
register', exception handler should restore emulated register state
while returning back, otherwise there will be register state
corruption. eg, adding a watchpoint on a list can corrput the list:

  # cat /proc/kallsyms | grep kthread_create_list
  c00000000121c8b8 d kthread_create_list

Add watchpoint on kthread_create_list->prev:

  # perf record -e mem:0xc00000000121c8c0

Run some workload such that new kthread gets invoked. eg, I just
logged out from console:

  list_add corruption. next->prev should be prev (c000000001214e00), \
	but was c00000000121c8b8. (next=c00000000121c8b8).
  WARNING: CPU: 59 PID: 309 at lib/list_debug.c:25 __list_add_valid+0xb4/0xc0
  CPU: 59 PID: 309 Comm: kworker/59:0 Kdump: loaded Not tainted 5.1.0-rc7+ #69
  ...
  NIP __list_add_valid+0xb4/0xc0
  LR __list_add_valid+0xb0/0xc0
  Call Trace:
  __list_add_valid+0xb0/0xc0 (unreliable)
  __kthread_create_on_node+0xe0/0x260
  kthread_create_on_node+0x34/0x50
  create_worker+0xe8/0x260
  worker_thread+0x444/0x560
  kthread+0x160/0x1a0
  ret_from_kernel_thread+0x5c/0x70

List corruption happened because it uses 'load into non-volatile
register' instruction:

Snippet from __kthread_create_on_node:

  c000000000136be8:     addis   r29,r2,-19
  c000000000136bec:     ld      r29,31424(r29)
        if (!__list_add_valid(new, prev, next))
  c000000000136bf0:     mr      r3,r30
  c000000000136bf4:     mr      r5,r28
  c000000000136bf8:     mr      r4,r29
  c000000000136bfc:     bl      c00000000059a2f8 <__list_add_valid+0x8>

Register state from WARN_ON():

  GPR00: c00000000059a3a0 c000007ff23afb50 c000000001344e00 0000000000000075
  GPR04: 0000000000000000 0000000000000000 0000001852af8bc1 0000000000000000
  GPR08: 0000000000000001 0000000000000007 0000000000000006 00000000000004aa
  GPR12: 0000000000000000 c000007ffffeb080 c000000000137038 c000005ff62aaa00
  GPR16: 0000000000000000 0000000000000000 c000007fffbe7600 c000007fffbe7370
  GPR20: c000007fffbe7320 c000007fffbe7300 c000000001373a00 0000000000000000
  GPR24: fffffffffffffef7 c00000000012e320 c000007ff23afcb0 c000000000cb8628
  GPR28: c00000000121c8b8 c000000001214e00 c000007fef5b17e8 c000007fef5b17c0

Watchpoint hit at 0xc000000000136bec.

  addis   r29,r2,-19
   => r29 = 0xc000000001344e00 + (-19 << 16)
   => r29 = 0xc000000001214e00

  ld      r29,31424(r29)
   => r29 = *(0xc000000001214e00 + 31424)
   => r29 = *(0xc00000000121c8c0)

0xc00000000121c8c0 is where we placed a watchpoint and thus this
instruction was emulated by emulate_step. But because handle_dabr_fault
did not restore emulated register state, r29 still contains stale
value in above register state.

Fixes: 5aae8a5 ("powerpc, hw_breakpoints: Implement hw_breakpoints for 64-bit server processors")
Signed-off-by: Ravi Bangoria <[email protected]>
Cc: [email protected] # 2.6.36+
Signed-off-by: Michael Ellerman <[email protected]>
nathanchance pushed a commit that referenced this issue Jul 24, 2019
On systems like P9 powernv where we have no TM (or P8 booted with
ppc_tm=off), userspace can construct a signal context which still has
the MSR TS bits set. The kernel tries to restore this context which
results in the following crash:

  Unexpected TM Bad Thing exception at c0000000000022fc (msr 0x8000000102a03031) tm_scratch=800000020280f033
  Oops: Unrecoverable exception, sig: 6 [#1]
  LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
  Modules linked in:
  CPU: 0 PID: 1636 Comm: sigfuz Not tainted 5.2.0-11043-g0a8ad0ffa4 #69
  NIP:  c0000000000022fc LR: 00007fffb2d67e48 CTR: 0000000000000000
  REGS: c00000003fffbd70 TRAP: 0700   Not tainted  (5.2.0-11045-g7142b497d8)
  MSR:  8000000102a03031 <SF,VEC,VSX,FP,ME,IR,DR,LE,TM[E]>  CR: 42004242  XER: 00000000
  CFAR: c0000000000022e0 IRQMASK: 0
  GPR00: 0000000000000072 00007fffb2b6e560 00007fffb2d87f00 0000000000000669
  GPR04: 00007fffb2b6e728 0000000000000000 0000000000000000 00007fffb2b6f2a8
  GPR08: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
  GPR12: 0000000000000000 00007fffb2b76900 0000000000000000 0000000000000000
  GPR16: 00007fffb2370000 00007fffb2d84390 00007fffea3a15ac 000001000a250420
  GPR20: 00007fffb2b6f260 0000000010001770 0000000000000000 0000000000000000
  GPR24: 00007fffb2d843a0 00007fffea3a14a0 0000000000010000 0000000000800000
  GPR28: 00007fffea3a14d8 00000000003d0f00 0000000000000000 00007fffb2b6e728
  NIP [c0000000000022fc] rfi_flush_fallback+0x7c/0x80
  LR [00007fffb2d67e48] 0x7fffb2d67e48
  Call Trace:
  Instruction dump:
  e96a0220 e96a02a8 e96a0330 e96a03b8 394a0400 4200ffdc 7d2903a6 e92d0c00
  e94d0c08 e96d0c10 e82d0c18 7db242a6 <4c000024> 7db243a6 7db142a6 f82d0c18

The problem is the signal code assumes TM is enabled when
CONFIG_PPC_TRANSACTIONAL_MEM is enabled. This may not be the case as
with P9 powernv or if `ppc_tm=off` is used on P8.

This means any local user can crash the system.

Fix the problem by returning a bad stack frame to the user if they try
to set the MSR TS bits with sigreturn() on systems where TM is not
supported.

Found with sigfuz kernel selftest on P9.

This fixes CVE-2019-13648.

Fixes: 2b0a576 ("powerpc: Add new transactional memory state to the signal context")
Cc: [email protected] # v3.9
Reported-by: Praveen Pandey <[email protected]>
Signed-off-by: Michael Neuling <[email protected]>
Signed-off-by: Michael Ellerman <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
nathanchance pushed a commit that referenced this issue Oct 13, 2020
In list_add, the first variable is the new node and the second
is the list head. The function is called with a wrong order causing
NULL dereference:

[   15.527030] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008
[   15.542317] Mem abort info:
[   15.545152]   ESR = 0x96000044
[   15.548248]   EC = 0x25: DABT (current EL), IL = 32 bits
[   15.553624]   SET = 0, FnV = 0
[   15.556715]   EA = 0, S1PTW = 0
[   15.559892] Data abort info:
[   15.562799]   ISV = 0, ISS = 0x00000044
[   15.566678]   CM = 0, WnR = 1
[   15.569683] user pgtable: 4k pages, 48-bit VAs, pgdp=00000001373f0000
[   15.576196] [0000000000000008] pgd=0000000000000000, p4d=0000000000000000
[   15.583101] Internal error: Oops: 96000044 [#1] PREEMPT SMP
[   15.588747] Modules linked in: mtk_mdp(+) cfg80211 v4l2_mem2mem videobuf2_vmalloc videobuf2_dma_contig videobuf2_memops videobuf2_v4l2 videobuf2_common vide
odev mt8173_rt5650 smsc95xx usbnet ecdh_generic ecc snd_soc_rt5645 mc mt8173_afe_pcm rfkill cros_ec_sensors snd_soc_mtk_common elan_i2c crct10dif_ce cros_ec_se
nsors_core snd_soc_rl6231 elants_i2c industrialio_triggered_buffer kfifo_buf mtk_vpu cros_ec_chardev cros_usbpd_charger cros_usbpd_logger sbs_battery display_c
onnector pwm_bl ip_tables x_tables ipv6
[   15.634295] CPU: 0 PID: 188 Comm: systemd-udevd Not tainted 5.9.0-rc2+ #69
[   15.641242] Hardware name: Google Elm (DT)
[   15.645381] pstate: 20000005 (nzCv daif -PAN -UAO BTYPE=--)
[   15.651022] pc : mtk_mdp_probe+0x134/0x3a8 [mtk_mdp]
[   15.656041] lr : mtk_mdp_probe+0x128/0x3a8 [mtk_mdp]
[   15.661055] sp : ffff80001255b910
[   15.669548] x29: ffff80001255b910 x28: 0000000000000000
[   15.679973] x27: ffff800009089bf8 x26: ffff0000fafde800
[   15.690347] x25: ffff0000ff7d2768 x24: ffff800009089010
[   15.700670] x23: ffff0000f01a7cd8 x22: ffff0000fafde810
[   15.710940] x21: ffff0000f01a7c80 x20: ffff0000f0c3c180
[   15.721148] x19: ffff0000ff7f1618 x18: 0000000000000010
[   15.731289] x17: 0000000000000000 x16: 0000000000000000
[   15.741375] x15: 0000000000aaaaaa x14: 0000000000000020
[   15.751399] x13: 00000000ffffffff x12: 0000000000000020
[   15.761363] x11: 0000000000000028 x10: 0101010101010101
[   15.771279] x9 : 0000000000000004 x8 : 7f7f7f7f7f7f7f7f
[   15.781148] x7 : 646bff6171606b2b x6 : 0000000000806d65
[   15.790981] x5 : ffff0000ff7f8360 x4 : 0000000000000000
[   15.800767] x3 : 0000000000000004 x2 : 0000000000000001
[   15.810501] x1 : 0000000000000005 x0 : 0000000000000000
[   15.820171] Call trace:
[   15.826944]  mtk_mdp_probe+0x134/0x3a8 [mtk_mdp]
[   15.835908]  platform_drv_probe+0x54/0xa8
[   15.844247]  really_probe+0xe4/0x3b0
[   15.852104]  driver_probe_device+0x58/0xb8
[   15.860457]  device_driver_attach+0x74/0x80
[   15.868854]  __driver_attach+0x58/0xe0
[   15.876770]  bus_for_each_dev+0x70/0xc0
[   15.884726]  driver_attach+0x24/0x30
[   15.892374]  bus_add_driver+0x14c/0x1f0
[   15.900295]  driver_register+0x64/0x120
[   15.908168]  __platform_driver_register+0x48/0x58
[   15.916864]  mtk_mdp_driver_init+0x20/0x1000 [mtk_mdp]
[   15.925943]  do_one_initcall+0x54/0x1b4
[   15.933662]  do_init_module+0x54/0x200
[   15.941246]  load_module+0x1cf8/0x22d0
[   15.948798]  __do_sys_finit_module+0xd8/0xf0
[   15.956829]  __arm64_sys_finit_module+0x20/0x30
[   15.965082]  el0_svc_common.constprop.0+0x6c/0x168
[   15.973527]  do_el0_svc+0x24/0x90
[   15.980403]  el0_sync_handler+0x90/0x198
[   15.987867]  el0_sync+0x158/0x180
[   15.994653] Code: 9400014b 2a0003fc 35000920 f9400280 (f9000417)
[   16.004299] ---[ end trace 76fee0203f9898e5 ]---

Fixes: 86698b9 ("media: mtk-mdp: convert mtk_mdp_dev.comp array to list")
Signed-off-by: Dafna Hirschfeld <[email protected]>
Reviewed-by: Matthias Brugger <[email protected]>
Tested-by: Enric Balletbo i Serra <[email protected]>
Signed-off-by: Hans Verkuil <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
nathanchance pushed a commit that referenced this issue Dec 16, 2020
 CHECK: Alignment should match open parenthesis
 #24: FILE: drivers/mfd/tps65910.c:296:
 +	ret = regmap_clear_bits(tps65910->regmap, TPS65910_DEVCTRL,
  						DEVCTRL_CK32K_CTRL_MASK);

 CHECK: Alignment should match open parenthesis
 #33: FILE: drivers/mfd/tps65910.c:318:
 +	ret = regmap_set_bits(tps65910->regmap, TPS65910_DEVCTRL,
  				DEVCTRL_DEV_SLP_MASK);

 CHECK: Alignment should match open parenthesis
 #42: FILE: drivers/mfd/tps65910.c:326:
 +		ret = regmap_set_bits(tps65910->regmap,
  				TPS65910_SLEEP_KEEP_RES_ON,

 CHECK: Alignment should match open parenthesis
 #51: FILE: drivers/mfd/tps65910.c:336:
 +		ret = regmap_set_bits(tps65910->regmap,
  				TPS65910_SLEEP_KEEP_RES_ON,

 CHECK: Alignment should match open parenthesis
 #60: FILE: drivers/mfd/tps65910.c:346:
 +		ret = regmap_set_bits(tps65910->regmap,
  				TPS65910_SLEEP_KEEP_RES_ON,

 CHECK: Alignment should match open parenthesis
 #69: FILE: drivers/mfd/tps65910.c:358:
 +	regmap_clear_bits(tps65910->regmap, TPS65910_DEVCTRL,
  				DEVCTRL_DEV_SLP_MASK);

 CHECK: Alignment should match open parenthesis
 #78: FILE: drivers/mfd/tps65910.c:440:
 +	if (regmap_set_bits(tps65910->regmap, TPS65910_DEVCTRL,
  			DEVCTRL_PWR_OFF_MASK) < 0)

 CHECK: Alignment should match open parenthesis
 #83: FILE: drivers/mfd/tps65910.c:444:
 +	regmap_clear_bits(tps65910->regmap, TPS65910_DEVCTRL,
  			DEVCTRL_DEV_ON_MASK);

Signed-off-by: Lee Jones <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-Wself-assign [BUG] linux A bug that should be fixed in the mainline kernel. [FIXED][LINUX] 4.20 This bug was fixed in Linux 4.20
Projects
None yet
Development

No branches or pull requests

3 participants