[PATCH net] macvlan: fix null pointer dereference in macvlan_changelink_sources()

[PATCH net] macvlan: fix null pointer dereference in macvlan_changelink_sources()

[wangyunjian]

From: Yunjian Wang <wangyunjian@huawei.com>

Currently pointer data is dereferenced when declaring addr before
pointer data is null checked. This could lead to a null pointer
dereference. Fix this by checking if pointer data is null first.

Fixes: 79cf79abce71 ("macvlan: add source mode")
Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
---
 drivers/net/macvlan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index fb51329f8964..e412fd6b6798 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -1356,7 +1356,7 @@ static int macvlan_changelink_sources(struct macvlan_dev *vlan, u32 mode,
 	struct nlattr *nla, *head;
 	struct macvlan_source_entry *entry;
 
-	if (data[IFLA_MACVLAN_MACADDR])
+	if (data && data[IFLA_MACVLAN_MACADDR])
 		addr = nla_data(data[IFLA_MACVLAN_MACADDR]);
 
 	if (mode == MACVLAN_MACADDR_ADD) {
-- 
2.23.0

Re: [PATCH net] macvlan: fix null pointer dereference in macvlan_changelink_sources()

[Jakub Kicinski]

On Wed, 30 Dec 2020 18:38:15 +0800 wangyunjian wrote:
> From: Yunjian Wang <wangyunjian@huawei.com>
> 
> Currently pointer data is dereferenced when declaring addr before
> pointer data is null checked. This could lead to a null pointer
> dereference. Fix this by checking if pointer data is null first.
> 
> Fixes: 79cf79abce71 ("macvlan: add source mode")
> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>

I don't see it. All calls to macvlan_changelink_sources() are under 
if (data) { ... } so data is never NULL. Looks like we should rather
clean up macvlan_changelink_sources() to not check data for
MACVLAN_MACADDR_SET. 

WDYT?

RE: [PATCH net] macvlan: fix null pointer dereference in macvlan_changelink_sources()

[wangyunjian]

> -----Original Message-----
> From: Jakub Kicinski [mailto:kuba@kernel.org]
> Sent: Tuesday, January 5, 2021 5:35 AM
> To: wangyunjian <wangyunjian@huawei.com>
> Cc: netdev@vger.kernel.org; davem@davemloft.net; Lilijun (Jerry)
> <jerry.lilijun@huawei.com>; xudingke <xudingke@huawei.com>
> Subject: Re: [PATCH net] macvlan: fix null pointer dereference in
> macvlan_changelink_sources()
> 
> On Wed, 30 Dec 2020 18:38:15 +0800 wangyunjian wrote:
> > From: Yunjian Wang <wangyunjian@huawei.com>
> >
> > Currently pointer data is dereferenced when declaring addr before
> > pointer data is null checked. This could lead to a null pointer
> > dereference. Fix this by checking if pointer data is null first.
> >
> > Fixes: 79cf79abce71 ("macvlan: add source mode")
> > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> 
> I don't see it. All calls to macvlan_changelink_sources() are under
> if (data) { ... } so data is never NULL. Looks like we should rather
> clean up macvlan_changelink_sources() to not check data for
> MACVLAN_MACADDR_SET.
> 
> WDYT?

OK, thanks for your suggestion, will include them in next version.

Yunjian