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

[Ben Hutchings]

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?

Ben.

-- 
Ben Hutchings
If you seem to know what you are doing, you'll be given more to do.

Attachment: signature.asc
Description: This is a digitally signed message part