vchiq: fix NULL pointer dereference when closing driver #1123
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 6 commits
September 2, 2015 11:50
The following code run as root will cause a null pointer dereference oops:
int fd = open("/dev/vc-cma", O_RDONLY);
if (fd < 0)
err(1, "open failed");
(void)close(fd);
[ 1704.877721] Unable to handle kernel NULL pointer dereference at virtual address 00000000
[ 1704.877725] pgd = b899c000
[ 1704.877736] [00000000] *pgd=37fab831, *pte=00000000, *ppte=00000000
[ 1704.877748] Internal error: Oops: 817 [#1] PREEMPT SMP ARM
[ 1704.877765] Modules linked in: evdev i2c_bcm2708 uio_pdrv_genirq uio
[ 1704.877774] CPU: 2 PID: 3656 Comm: stress-ng-fstat Not tainted 3.19.1-12-generic-bcm2709 #12-Ubuntu
[ 1704.877777] Hardware name: BCM2709
[ 1704.877783] task: b8ab9b00 ti: b7e68000 task.ti: b7e68000
[ 1704.877798] PC is at __down_interruptible+0x50/0xec
[ 1704.877806] LR is at down_interruptible+0x5c/0x68
[ 1704.877813] pc : [<80630ee8>] lr : [<800704b0>] psr: 60080093
sp : b7e69e50 ip : b7e69e88 fp : b7e69e84
[ 1704.877817] r10: b88123c8 r9 : 00000010 r8 : 00000001
[ 1704.877822] r7 : b8ab9b00 r6 : 7fffffff r5 : 80a1cc34 r4 : 80a1cc34
[ 1704.877826] r3 : b7e69e50 r2 : 00000000 r1 : 00000000 r0 : 80a1cc34
[ 1704.877833] Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment user
[ 1704.877838] Control: 10c5387d Table: 3899c06a DAC: 00000015
[ 1704.877843] Process do-oops (pid: 3656, stack limit = 0xb7e68238)
[ 1704.877848] Stack: (0xb7e69e50 to 0xb7e6a000)
[ 1704.877856] 9e40: 80a1cc3c 00000000 00000010 b88123c8
[ 1704.877865] 9e60: b7e69e84 80a1cc34 fff9fee9 ffffffff b7e68000 00000009 b7e69ea4 b7e69e88
[ 1704.877874] 9e80: 800704b0 80630ea4 fff9fee9 60080013 80a1cc28 fff9fee9 b7e69edc b7e69ea8
[ 1704.877884] 9ea0: 8040f558 80070460 fff9fee9 ffffffff 00000000 00000000 00000009 80a1cb7c
[ 1704.877893] 9ec0: 00000000 80a1cb7c 00000000 00000010 b7e69ef4 b7e69ee0 803e1ba4 8040f514
[ 1704.877902] 9ee0: 00000e48 80a1cb7c b7e69f14 b7e69ef8 803e1c9c 803e1b74 b88123c0 b92acb18
[ 1704.877911] 9f00: b8812790 b8d815d8 b7e69f24 b7e69f18 803e2250 803e1bc8 b7e69f5c b7e69f28
[ 1704.877921] 9f20: 80167bac 803e222c 00000000 00000000 b7e69f54 b8ab9ffc 00000000 8098c794
[ 1704.877930] 9f40: b8ab9b00 8000efc4 b7e68000 00000000 b7e69f6c b7e69f60 80167d6c 80167b28
[ 1704.877939] 9f60: b7e69f8c b7e69f70 80047d38 80167d60 b7e68000 b7e68010 8000efc4 b7e69fb0
[ 1704.877949] 9f80: b7e69fac b7e69f90 80012820 80047c84 01155490 011549a8 00000001 00000006
[ 1704.877957] 9fa0: 00000000 b7e69fb0 8000ee5c 80012790 00000000 353d8c0f 7efc4308 00000000
[ 1704.877966] 9fc0: 01155490 011549a8 00000001 00000006 00000000 00000000 76cf3ba0 00000003
[ 1704.877975] 9fe0: 00000000 7efc42e4 0002272f 76e2ed66 60080030 00000003 00000000 00000000
[ 1704.877998] [<80630ee8>] (__down_interruptible) from [<800704b0>] (down_interruptible+0x5c/0x68)
[ 1704.878015] [<800704b0>] (down_interruptible) from [<8040f558>] (vchiu_queue_push+0x50/0xd8)
[ 1704.878032] [<8040f558>] (vchiu_queue_push) from [<803e1ba4>] (send_worker_msg+0x3c/0x54)
[ 1704.878045] [<803e1ba4>] (send_worker_msg) from [<803e1c9c>] (vc_cma_set_reserve+0xe0/0x1c4)
[ 1704.878057] [<803e1c9c>] (vc_cma_set_reserve) from [<803e2250>] (vc_cma_release+0x30/0x38)
[ 1704.878069] [<803e2250>] (vc_cma_release) from [<80167bac>] (__fput+0x90/0x1e0)
[ 1704.878082] [<80167bac>] (__fput) from [<80167d6c>] (____fput+0x18/0x1c)
[ 1704.878094] [<80167d6c>] (____fput) from [<80047d38>] (task_work_run+0xc0/0xf8)
[ 1704.878109] [<80047d38>] (task_work_run) from [<80012820>] (do_work_pending+0x9c/0xc4)
[ 1704.878123] [<80012820>] (do_work_pending) from [<8000ee5c>] (work_pending+0xc/0x20)
[ 1704.878133] Code: e50b1034 e3a01000 e50b2030 e580300c (e5823000)
..the fix is to ensure that we have actually initialized the queue before we attempt
to push any items onto it. This occurs if we do an open() followed by a close() without
any activity in between.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
static analysis with cppcheck detected some redundant code which should probably be removed: [drivers/video/fbdev/bcm2708_fb.c:269]: (style) Variable 'yres' is assigned a value that is never used. Signed-off-by: Colin Ian King <colin.king@canonical.com>
static analysis by cppcheck detected an unnecessary initialization of variable 'result' which is re-assigned almost immediately after the initialization. Remove the redundant initialization. [drivers/video/fbdev/bcm2708_fb.c:406]: (performance) Variable 'result' is reassigned a value before the old one has been used. Signed-off-by: Colin Ian King <colin.king@canonical.com>
…e the resource resource can be kfree'd when the reference count is zero, so we should not bump the res_stats of the resource after the vmcs_sm_release_resource call since the resource may have been kfree'd by this call. Instead, bump the stats before we call vmcs_sm_release_resource to avoid a potential NULL pointer derefernce. Bug found using cppcheck static analysis: [drivers/char/broadcom/vc_sm/vmcs_sm.c:1373]: (error) Dereferencing 'resource' after it is deallocated / released Signed-off-by: Colin Ian King <colin.king@canonical.com>
static analysis by cppcheck detected some potential NULL pointer dereference issues: [drivers/media/platform/bcm2835/controls.c:854]: (error) Possible null pointer dereference: scene (and lines 858, 859 too) it is possible that scene is not found because of an invalue ctrl->val and is therefore NULL and hence causing a null pointer dereference. Signed-off-by: Colin Ian King <colin.king@canonical.com>
static analysis by cppcheck detected a memcpy to rmsg which is not actually initialized at that point. The memcpy should be copying to variable m instead. Signed-off-by: Colin Ian King <colin.king@canonical.com>
|
Various driver related fixes to issues found using cppcheck static analysis |
pelwell
added a commit
that referenced
this pull request
Sep 11, 2015
vchiq: fix NULL pointer dereference when closing driver
popcornmix
added a commit
to raspberrypi/firmware
that referenced
this pull request
Sep 14, 2015
kernel: Revert "BCM270X_DT: mz61581: Revert to spi-bcm2708" See: raspberrypi/linux#1132 kernel: bcm2835-mmc: Don't overwrite MMC capabilities from DT kernel: BCM270X_DT: Use fixed-factor-clock for uart1 See: raspberrypi/linux#1008 kernel: vchiq: fix NULL pointer dereference when closing driver See: raspberrypi/linux#1123 firmware: Fix touchscreen I2C to only read from i2c in smaller bursts to avoid a fifo overrun problem with the i2c peripheral See: https://www.raspberrypi.org/forums/viewtopic.php?f=43&t=120642 firmware: arm_display: Fix issue with nonsensical negative overscan settings See: #471 firmware: arm_loader: Enable the i2c_arm and i2c_vc aliases for CM See: raspberrypi/linux#1129 firmware: di_adv: Allow the v3d priority boost to be modified See: http://forum.kodi.tv/showthread.php?tid=231092&pid=2103200#pid2103200
popcornmix
added a commit
to Hexxeh/rpi-firmware
that referenced
this pull request
Sep 14, 2015
kernel: Revert "BCM270X_DT: mz61581: Revert to spi-bcm2708" See: raspberrypi/linux#1132 kernel: bcm2835-mmc: Don't overwrite MMC capabilities from DT kernel: BCM270X_DT: Use fixed-factor-clock for uart1 See: raspberrypi/linux#1008 kernel: vchiq: fix NULL pointer dereference when closing driver See: raspberrypi/linux#1123 firmware: Fix touchscreen I2C to only read from i2c in smaller bursts to avoid a fifo overrun problem with the i2c peripheral See: https://www.raspberrypi.org/forums/viewtopic.php?f=43&t=120642 firmware: arm_display: Fix issue with nonsensical negative overscan settings See: raspberrypi/firmware#471 firmware: arm_loader: Enable the i2c_arm and i2c_vc aliases for CM See: raspberrypi/linux#1129 firmware: di_adv: Allow the v3d priority boost to be modified See: http://forum.kodi.tv/showthread.php?tid=231092&pid=2103200#pid2103200
neuschaefer
pushed a commit
to neuschaefer/raspi-binary-firmware
that referenced
this pull request
Feb 27, 2017
kernel: Revert "BCM270X_DT: mz61581: Revert to spi-bcm2708" See: raspberrypi/linux#1132 kernel: bcm2835-mmc: Don't overwrite MMC capabilities from DT kernel: BCM270X_DT: Use fixed-factor-clock for uart1 See: raspberrypi/linux#1008 kernel: vchiq: fix NULL pointer dereference when closing driver See: raspberrypi/linux#1123 firmware: Fix touchscreen I2C to only read from i2c in smaller bursts to avoid a fifo overrun problem with the i2c peripheral See: https://www.raspberrypi.org/forums/viewtopic.php?f=43&t=120642 firmware: arm_display: Fix issue with nonsensical negative overscan settings See: raspberrypi#471 firmware: arm_loader: Enable the i2c_arm and i2c_vc aliases for CM See: raspberrypi/linux#1129 firmware: di_adv: Allow the v3d priority boost to be modified See: http://forum.kodi.tv/showthread.php?tid=231092&pid=2103200#pid2103200
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The following code run as root will cause a null pointer dereference oops:
[ 1704.877721] Unable to handle kernel NULL pointer dereference at virtual address 00000000
[ 1704.877725] pgd = b899c000
[ 1704.877736] [00000000] *pgd=37fab831, *pte=00000000, *ppte=00000000
[ 1704.877748] Internal error: Oops: 817 [#1] PREEMPT SMP ARM
[ 1704.877765] Modules linked in: evdev i2c_bcm2708 uio_pdrv_genirq uio
[ 1704.877774] CPU: 2 PID: 3656 Comm: stress-ng-fstat Not tainted 3.19.1-12-generic-bcm2709 #12-Ubuntu
[ 1704.877777] Hardware name: BCM2709
[ 1704.877783] task: b8ab9b00 ti: b7e68000 task.ti: b7e68000
[ 1704.877798] PC is at __down_interruptible+0x50/0xec
[ 1704.877806] LR is at down_interruptible+0x5c/0x68
[ 1704.877813] pc : [<80630ee8>] lr : [<800704b0>] psr: 60080093
sp : b7e69e50 ip : b7e69e88 fp : b7e69e84
[ 1704.877817] r10: b88123c8 r9 : 00000010 r8 : 00000001
[ 1704.877822] r7 : b8ab9b00 r6 : 7fffffff r5 : 80a1cc34 r4 : 80a1cc34
[ 1704.877826] r3 : b7e69e50 r2 : 00000000 r1 : 00000000 r0 : 80a1cc34
[ 1704.877833] Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment user
[ 1704.877838] Control: 10c5387d Table: 3899c06a DAC: 00000015
[ 1704.877843] Process do-oops (pid: 3656, stack limit = 0xb7e68238)
[ 1704.877848] Stack: (0xb7e69e50 to 0xb7e6a000)
[ 1704.877856] 9e40: 80a1cc3c 00000000 00000010 b88123c8
[ 1704.877865] 9e60: b7e69e84 80a1cc34 fff9fee9 ffffffff b7e68000 00000009 b7e69ea4 b7e69e88
[ 1704.877874] 9e80: 800704b0 80630ea4 fff9fee9 60080013 80a1cc28 fff9fee9 b7e69edc b7e69ea8
[ 1704.877884] 9ea0: 8040f558 80070460 fff9fee9 ffffffff 00000000 00000000 00000009 80a1cb7c
[ 1704.877893] 9ec0: 00000000 80a1cb7c 00000000 00000010 b7e69ef4 b7e69ee0 803e1ba4 8040f514
[ 1704.877902] 9ee0: 00000e48 80a1cb7c b7e69f14 b7e69ef8 803e1c9c 803e1b74 b88123c0 b92acb18
[ 1704.877911] 9f00: b8812790 b8d815d8 b7e69f24 b7e69f18 803e2250 803e1bc8 b7e69f5c b7e69f28
[ 1704.877921] 9f20: 80167bac 803e222c 00000000 00000000 b7e69f54 b8ab9ffc 00000000 8098c794
[ 1704.877930] 9f40: b8ab9b00 8000efc4 b7e68000 00000000 b7e69f6c b7e69f60 80167d6c 80167b28
[ 1704.877939] 9f60: b7e69f8c b7e69f70 80047d38 80167d60 b7e68000 b7e68010 8000efc4 b7e69fb0
[ 1704.877949] 9f80: b7e69fac b7e69f90 80012820 80047c84 01155490 011549a8 00000001 00000006
[ 1704.877957] 9fa0: 00000000 b7e69fb0 8000ee5c 80012790 00000000 353d8c0f 7efc4308 00000000
[ 1704.877966] 9fc0: 01155490 011549a8 00000001 00000006 00000000 00000000 76cf3ba0 00000003
[ 1704.877975] 9fe0: 00000000 7efc42e4 0002272f 76e2ed66 60080030 00000003 00000000 00000000
[ 1704.877998] <80630ee8> from <800704b0>
[ 1704.878015] <800704b0> from <8040f558>
[ 1704.878032] <8040f558> from <803e1ba4>
[ 1704.878045] <803e1ba4> from <803e1c9c>
[ 1704.878057] <803e1c9c> from <803e2250>
[ 1704.878069] <803e2250> from <80167bac>
[ 1704.878082] <80167bac> from <80167d6c>
[ 1704.878094] <80167d6c> from <80047d38>
[ 1704.878109] <80047d38> from <80012820>
[ 1704.878123] <80012820> from <8000ee5c>
[ 1704.878133] Code: e50b1034 e3a01000 e50b2030 e580300c (e5823000)
..the fix is to ensure that we have actually initialized the queue before we attempt
to push any items onto it. This occurs if we do an open() followed by a close() without
any activity in between.
Signed-off-by: Colin Ian King colin.king@canonical.com