Messages in this thread | | | Subject | Re: [PATCH] staging: most: don't access hdm_ch before checking it valid | From | Jing Xiangfeng <> | Date | Tue, 29 Sep 2020 09:01:14 +0800 |
| |
On 2020/9/28 19:48, Dan Carpenter wrote: > On Mon, Sep 28, 2020 at 06:48:38PM +0800, Jing Xiangfeng wrote: >> In try_start_dim_transfer(), pointer hdm_ch is accessed before checking. >> This may lead to a potential null pointer dereference. Fix this by >> dereferencing hdm_ch after calling BUG_ON(). >> >> Signed-off-by: Jing Xiangfeng <jingxiangfeng@huawei.com> >> --- >> drivers/staging/most/dim2/dim2.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c >> index 509c8012d20b..ccd7cc7545e4 100644 >> --- a/drivers/staging/most/dim2/dim2.c >> +++ b/drivers/staging/most/dim2/dim2.c >> @@ -148,7 +148,7 @@ void dimcb_on_error(u8 error_id, const char *error_message) >> static int try_start_dim_transfer(struct hdm_channel *hdm_ch) >> { >> u16 buf_size; >> - struct list_head *head = &hdm_ch->pending_list; > This is not a dereference, it's just pointer math. In other words: > > struct list_head *head = hdm_ch + offsetof(struct hdm_channel, pending_list); Thanks for correcting!
> > So the commit message is wrong because this cannot lead to a NULL > dereference. It's better to just delete the BUG_ON(). We don't really > like BUG_ON(). Checkpatch will complain about them. An Oops gives > basically the same information as a BUG_ON() without completely killing > the kernel so just dereferencing a NULL is preferable. Finally, we can > see from the callers that "hdm_ch" is never NULL. > > regards, > dan carpenter > > . >
| |