Re: [PATCH 3.2 31/34] rtl8192ce: Fix null dereference in watchdog

From: Larry Finger
Date: Fri May 16 2014 - 12:08:53 EST


On 05/16/2014 10:38 AM, Ben Hutchings wrote:
On Fri, 2014-05-16 at 09:20 -0500, Larry Finger wrote:
On 05/16/2014 07:47 AM, Ben Hutchings wrote:
3.2.59-rc1 review patch. If anyone has any objections, please let me know.

------------------

From: Ben Hutchings <ben@xxxxxxxxxxxxxxx>

Dmitry Semyonov reported that after upgrading from 3.2.54 to
3.2.57 the rtl8192ce driver will crash when its interface is brought
up. The oops message shows:

[ 1833.611397] BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
[ 1833.611455] IP: [<ffffffffa0410c6a>] rtl92ce_update_hal_rate_tbl+0x29/0x4db [rtl8192ce]
...
[ 1833.613326] Call Trace:
[ 1833.613346] [<ffffffffa02ad9c6>] ? rtl92c_dm_watchdog+0xd0b/0xec9 [rtl8192c_common]
[ 1833.613391] [<ffffffff8105b5cf>] ? process_one_work+0x161/0x269
[ 1833.613425] [<ffffffff8105c598>] ? worker_thread+0xc2/0x145
[ 1833.613458] [<ffffffff8105c4d6>] ? manage_workers.isra.25+0x15b/0x15b
[ 1833.613496] [<ffffffff8105f6d9>] ? kthread+0x76/0x7e
[ 1833.613527] [<ffffffff81356b74>] ? kernel_thread_helper+0x4/0x10
[ 1833.613563] [<ffffffff8105f663>] ? kthread_worker_fn+0x139/0x139
[ 1833.613598] [<ffffffff81356b70>] ? gs_change+0x13/0x13

Disassembly of rtl92ce_update_hal_rate_tbl() shows that the 'sta'
parameter was null. None of the changes to the rtlwifi family between
3.2.54 and 3.2.57 seem to directly cause this, and reverting commit
f78bccd79ba3 ('rtlwifi: rtl8192ce: Fix too long disable of IRQs')
doesn't fix it.

rtl92c_dm_watchdog() calls rtl92ce_update_hal_rate_tbl() via
rtl92c_dm_refresh_rate_adaptive_mask(), which does not appear in the
call trace as it was inlined. That function has been completely
removed upstream which may explain why this crash wasn't seen there.

I'm not sure that it is sensible to completely remove
rtl92c_dm_refresh_rate_adaptive_mask() without making other
compensating changes elsewhere, so try to work around this for 3.2 by
checking for a null pointer in rtl92c_dm_refresh_rate_adaptive_mask()
and then skipping the call to rtl92ce_update_hal_rate_tbl().

References: https://bugs.debian.org/745137
References: https://bugs.debian.org/745462
Reported-by: Dmitry Semyonov <linulin@xxxxxxxxx>
Signed-off-by: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
Cc: Larry Finger <Larry.Finger@xxxxxxxxxxxx>
Cc: Chaoming Li <chaoming_li@xxxxxxxxxxxxxx>
---

Ben,

Your fix is a reasonable workaround. I have no explanation for this NULL pointer
dereference to suddenly appear; however, the pointer should have been checked
from the start.

Are you saying this is also an upstream bug?

It is a possible upstream bug; however, crashes of the form reported by Dmitri have not been reported to me, but then I was not aware of his problem. A quick check shows that the pointers returned by ieee80211_find_sta() are checked in current mainline. There are some cases that need a second look. I will be submitting a patch to upstream and stable for any of them where the checking was missed.

Larry

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/