RDMA/nldev: Fix multiple potential NULL pointer dereferences

PATCH In review Gustavo A. R. Silva 3 participants 8 messages Feb 9, 2018 – Feb 12, 2018 InfiniBand

lore

Share & tools

lore

Gustavo A. R. Silva

author

2018-02-09 06:37 UTC #0 lore raw

PATCH 1/1 RDMA/nldev: Fix multiple potential NULL pointer dereferences

In case the message header and payload cannot be stored, function nlmsg_put returns null.

Fix this by adding multiple sanity checks and avoid a potential null dereference on _nlh_ when calling nlmsg_end.

Addresses-Coverity-ID: 1454215 ("Dereference null return value")
Addresses-Coverity-ID: 1454223 ("Dereference null return value")
Addresses-Coverity-ID: 1454224 ("Dereference null return value")
Addresses-Coverity-ID: 1464669 ("Dereference null return value")
Addresses-Coverity-ID: 1464670 ("Dereference null return value")
Addresses-Coverity-ID: 1464672 ("Dereference null return value")

Fixes e5c9469efcb1 ("RDMA/netlink: Add nldev device doit implementation")

Fixes c3f66f7b0052 ("RDMA/netlink: Implement nldev port doit callback")

Fixes 7d02f605f0dc ("RDMA/netlink: Add nldev port dumpit implementation")

Fixes b5fa635aab8f ("RDMA/nldev: Provide detailed QP information")

Fixes bf3c5a93c523 ("RDMA/nldev: Provide global resource utilization")

Signed-off-by Gustavo A. R. Silva <[email protected]>

---
 drivers/infiniband/core/nldev.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

drivers/infiniband/core/nldev.c +25 -1

index 5326a68..dc8f6eb 100644
@@ -313,6 +313,11 @@ static int nldev_get_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
 	nlh = nlmsg_put(msg, NETLINK_CB(skb).portid, nlh->nlmsg_seq,
 			RDMA_NL_GET_TYPE(RDMA_NL_NLDEV, RDMA_NLDEV_CMD_GET),
 			0, 0);
+	if (!nlh) {
+		err = -EMSGSIZE;
+		goto err_free;
+
+	}
 
 	err = fill_dev_info(msg, device);
 	if (err)

@@ -344,6 +349,8 @@ static int _nldev_get_dumpit(struct ib_device *device,
 	nlh = nlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
 			RDMA_NL_GET_TYPE(RDMA_NL_NLDEV, RDMA_NLDEV_CMD_GET),
 			0, NLM_F_MULTI);
+	if (!nlh)
+		goto out;
 
 	if (fill_dev_info(skb, device)) {
 		nlmsg_cancel(skb, nlh);

@@ -354,7 +361,8 @@ static int _nldev_get_dumpit(struct ib_device *device,
 
 	idx++;
 
-out:	cb->args[0] = idx;
+out:
+	cb->args[0] = idx;
 	return skb->len;
 }

@@ -404,6 +412,10 @@ static int nldev_port_get_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
 	nlh = nlmsg_put(msg, NETLINK_CB(skb).portid, nlh->nlmsg_seq,
 			RDMA_NL_GET_TYPE(RDMA_NL_NLDEV, RDMA_NLDEV_CMD_GET),
 			0, 0);
+	if (!nlh) {
+		err = -EMSGSIZE;
+		goto err_free;
+	}
 
 	err = fill_port_info(msg, device, port);
 	if (err)

@@ -464,6 +476,8 @@ static int nldev_port_get_dumpit(struct sk_buff *skb,
 				RDMA_NL_GET_TYPE(RDMA_NL_NLDEV,
 						 RDMA_NLDEV_CMD_PORT_GET),
 				0, NLM_F_MULTI);
+		if (!nlh)
+			goto out;
 
 		if (fill_port_info(skb, device, p)) {
 			nlmsg_cancel(skb, nlh);

@@ -507,6 +521,10 @@ static int nldev_res_get_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
 	nlh = nlmsg_put(msg, NETLINK_CB(skb).portid, nlh->nlmsg_seq,
 			RDMA_NL_GET_TYPE(RDMA_NL_NLDEV, RDMA_NLDEV_CMD_RES_GET),
 			0, 0);
+	if (!nlh) {
+		ret = -EMSGSIZE;
+		goto err_free;
+	}
 
 	ret = fill_res_info(msg, device);
 	if (ret)

@@ -537,6 +555,8 @@ static int _nldev_res_get_dumpit(struct ib_device *device,
 	nlh = nlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
 			RDMA_NL_GET_TYPE(RDMA_NL_NLDEV, RDMA_NLDEV_CMD_RES_GET),
 			0, NLM_F_MULTI);
+	if (!nlh)
+		goto out;
 
 	if (fill_res_info(skb, device)) {
 		nlmsg_cancel(skb, nlh);

@@ -603,6 +623,10 @@ static int nldev_res_get_qp_dumpit(struct sk_buff *skb,
 	nlh = nlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
 			RDMA_NL_GET_TYPE(RDMA_NL_NLDEV, RDMA_NLDEV_CMD_RES_QP_GET),
 			0, NLM_F_MULTI);
+	if (!nlh) {
+		ret = -EMSGSIZE;
+		goto err_index;
+	}
 
 	if (fill_nldev_handle(skb, device)) {
 		ret = -EMSGSIZE;

Signature · 1 line

2.7.4

Leon Romanovsky

2018-02-09 12:25 UTC #1 lore raw

Quoted 18 lines · Gustavo A. R. Silva

In case the message header and payload cannot be stored, function nlmsg_put returns null.
Fix this by adding multiple sanity checks and avoid a potential null dereference on _nlh_ when calling nlmsg_end.
Addresses-Coverity-ID: 1454215 ("Dereference null return value")
Addresses-Coverity-ID: 1454223 ("Dereference null return value")
Addresses-Coverity-ID: 1454224 ("Dereference null return value")
Addresses-Coverity-ID: 1464669 ("Dereference null return value")
Addresses-Coverity-ID: 1464670 ("Dereference null return value")
Addresses-Coverity-ID: 1464672 ("Dereference null return value")
Fixes e5c9469efcb1 ("RDMA/netlink: Add nldev device doit implementation")
Fixes c3f66f7b0052 ("RDMA/netlink: Implement nldev port doit callback")
Fixes 7d02f605f0dc ("RDMA/netlink: Add nldev port dumpit implementation")
Fixes b5fa635aab8f ("RDMA/nldev: Provide detailed QP information")
Fixes bf3c5a93c523 ("RDMA/nldev: Provide global resource utilization")
Signed-off-by Gustavo A. R. Silva <[email protected]>

Quoted 4 lines

---
 drivers/infiniband/core/nldev.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

It will be much better to fix the tool instead of fixing ghost case. This scenario is impossible for all those flows.
We can receive the skv/msg in two ways:
* First by allocating new message with NLMSG_DEFAULT_SIZE, which has more room than nlmsg_total_size(payload), payload is 0.
* Second by getting from netlink.c and it will be at least "struct nlmsghdr" too.

Can you please add this info to the commit message?

Thanks

Gustavo A. R. Silva

2018-02-09 13:36 UTC #2 lore raw

Hi Leon,

Quoting Leon Romanovsky <[email protected]>:

Quoted 33 lines · Gustavo A. R. Silva

Quoted 18 lines
In case the message header and payload cannot be stored, function nlmsg_put returns null.
Fix this by adding multiple sanity checks and avoid a potential null dereference on _nlh_ when calling nlmsg_end.
Addresses-Coverity-ID: 1454215 ("Dereference null return value")
Addresses-Coverity-ID: 1454223 ("Dereference null return value")
Addresses-Coverity-ID: 1454224 ("Dereference null return value")
Addresses-Coverity-ID: 1464669 ("Dereference null return value")
Addresses-Coverity-ID: 1464670 ("Dereference null return value")
Addresses-Coverity-ID: 1464672 ("Dereference null return value")
Fixes e5c9469efcb1 ("RDMA/netlink: Add nldev device doit implementation")
Fixes c3f66f7b0052 ("RDMA/netlink: Implement nldev port doit callback")
Fixes 7d02f605f0dc ("RDMA/netlink: Add nldev port dumpit implementation")
Fixes b5fa635aab8f ("RDMA/nldev: Provide detailed QP information")
Fixes bf3c5a93c523 ("RDMA/nldev: Provide global resource utilization")
Signed-off-by Gustavo A. R. Silva <[email protected]>
Quoted 4 lines
---
 drivers/infiniband/core/nldev.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)
It will be much better to fix the tool instead of fixing ghost case. This scenario is impossible for all those flows.
We can receive the skv/msg in two ways:
* First by allocating new message with NLMSG_DEFAULT_SIZE, which
has more room
than nlmsg_total_size(payload), payload is 0.
* Second by getting from netlink.c and it will be at least "struct
nlmsghdr" too.

Quoted 2 lines

Can you please add this info to the commit message?

Actually, I was planing to send a new version of this patch. This time using the unlikely macro for all the null checks on nlh.

What do you think?

Thanks
--
Gustavo

Leon Romanovsky

2018-02-09 14:35 UTC #3 lore raw

Quoted 37 lines · Gustavo A. R. Silva

Hi Leon,
Quoting Leon Romanovsky <[email protected]>:
Quoted 33 lines · Gustavo A. R. Silva
Quoted 18 lines
In case the message header and payload cannot be stored, function nlmsg_put returns null.
Fix this by adding multiple sanity checks and avoid a potential null dereference on _nlh_ when calling nlmsg_end.
Addresses-Coverity-ID: 1454215 ("Dereference null return value")
Addresses-Coverity-ID: 1454223 ("Dereference null return value")
Addresses-Coverity-ID: 1454224 ("Dereference null return value")
Addresses-Coverity-ID: 1464669 ("Dereference null return value")
Addresses-Coverity-ID: 1464670 ("Dereference null return value")
Addresses-Coverity-ID: 1464672 ("Dereference null return value")
Fixes e5c9469efcb1 ("RDMA/netlink: Add nldev device doit implementation")
Fixes c3f66f7b0052 ("RDMA/netlink: Implement nldev port doit callback")
Fixes 7d02f605f0dc ("RDMA/netlink: Add nldev port dumpit implementation")
Fixes b5fa635aab8f ("RDMA/nldev: Provide detailed QP information")
Fixes bf3c5a93c523 ("RDMA/nldev: Provide global resource utilization")
Signed-off-by Gustavo A. R. Silva <gustavo-L1vi/[email protected]>
Quoted 4 lines
---
 drivers/infiniband/core/nldev.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)
It will be much better to fix the tool instead of fixing ghost case. This scenario is impossible for all those flows.
We can receive the skv/msg in two ways:
* First by allocating new message with NLMSG_DEFAULT_SIZE, which has
more room
than nlmsg_total_size(payload), payload is 0.
* Second by getting from netlink.c and it will be at least "struct
nlmsghdr" too.

Quoted 7 lines

Quoted 2 lines
Can you please add this info to the commit message?
Actually, I was planing to send a new version of this patch. This time using the unlikely macro for all the null checks on nlh.
What do you think?

It is not datapath, so "unlikely" is not needed. Let's assume that smart enough compiler will optimize such flow anyway, because nlmsg_put returns NULL in unlikely scenario, so this check will be unlikely automatically too.

Thanks

Quoted 10 lines

Thanks
--
Gustavo

Gustavo A. R. Silva

2018-02-09 15:56 UTC #4 lore raw

Quoting Leon Romanovsky <[email protected]>:

Quoted 43 lines · Gustavo A. R. Silva

Quoted 42 lines
Hi Leon,
Quoting Leon Romanovsky <[email protected]>:
Quoted 7 lines · Gustavo A. R. Silva
Quoted 6 lines
In case the message header and payload cannot be stored, function nlmsg_put returns null.
Fix this by adding multiple sanity checks and avoid a potential null dereference on _nlh_ when calling nlmsg_end.
Quoted 7 lines
Quoted 7 lines
Addresses-Coverity-ID: 1454215 ("Dereference null return value")
Addresses-Coverity-ID: 1454223 ("Dereference null return value")
Addresses-Coverity-ID: 1454224 ("Dereference null return value")
Addresses-Coverity-ID: 1464669 ("Dereference null return value")
Addresses-Coverity-ID: 1464670 ("Dereference null return value")
Addresses-Coverity-ID: 1464672 ("Dereference null return value")
Fixes e5c9469efcb1 ("RDMA/netlink: Add nldev device doit
implementation")
Quoted 2 lines
Quoted 2 lines
Fixes c3f66f7b0052 ("RDMA/netlink: Implement nldev port doit callback")
Fixes 7d02f605f0dc ("RDMA/netlink: Add nldev port dumpit
implementation")
Quoted 19 lines
Quoted 7 lines
Fixes b5fa635aab8f ("RDMA/nldev: Provide detailed QP information")
Fixes bf3c5a93c523 ("RDMA/nldev: Provide global resource utilization")
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/infiniband/core/nldev.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)
It will be much better to fix the tool instead of fixing ghost case. This scenario is impossible for all those flows.
We can receive the skv/msg in two ways:
* First by allocating new message with NLMSG_DEFAULT_SIZE, which has
more room
than nlmsg_total_size(payload), payload is 0.
* Second by getting from netlink.c and it will be at least "struct
nlmsghdr" too.
Can you please add this info to the commit message?

Quoted 9 lines

Quoted 4 lines
Actually, I was planing to send a new version of this patch. This time using the unlikely macro for all the null checks on nlh.
What do you think?
It is not datapath, so "unlikely" is not needed. Let's assume that smart enough compiler will optimize such flow anyway, because nlmsg_put returns NULL in unlikely scenario, so this check will be unlikely automatically too.

I'm curious about why the return value of nlmsg_put is null checked 118 out of 129 times (based on Coverity reports) in the last linux-next tree.

So based on what you mention, do you think all those checks are actually unnecessary and, maybe they should be removed?

Thanks
--
Gustavo

Leon Romanovsky

2018-02-09 16:49 UTC #5 lore raw

Quoted 57 lines · Gustavo A. R. Silva

Quoting Leon Romanovsky <[email protected]>:
Quoted 46 lines · Gustavo A. R. Silva
Quoted 45 lines
Hi Leon,
Quoting Leon Romanovsky <[email protected]>:
Quoted 7 lines · Gustavo A. R. Silva
Quoted 6 lines
In case the message header and payload cannot be stored, function nlmsg_put returns null.
Fix this by adding multiple sanity checks and avoid a potential null dereference on _nlh_ when calling nlmsg_end.
Quoted 7 lines
Quoted 7 lines
Addresses-Coverity-ID: 1454215 ("Dereference null return value")
Addresses-Coverity-ID: 1454223 ("Dereference null return value")
Addresses-Coverity-ID: 1454224 ("Dereference null return value")
Addresses-Coverity-ID: 1464669 ("Dereference null return value")
Addresses-Coverity-ID: 1464670 ("Dereference null return value")
Addresses-Coverity-ID: 1464672 ("Dereference null return value")
Fixes e5c9469efcb1 ("RDMA/netlink: Add nldev device doit
implementation")
Quoted 2 lines
Quoted 2 lines
Fixes c3f66f7b0052 ("RDMA/netlink: Implement nldev port doit callback")
Fixes 7d02f605f0dc ("RDMA/netlink: Add nldev port dumpit
implementation")
Quoted 17 lines
Quoted 7 lines
Fixes b5fa635aab8f ("RDMA/nldev: Provide detailed QP information")
Fixes bf3c5a93c523 ("RDMA/nldev: Provide global resource utilization")
Signed-off-by Gustavo A. R. Silva <gustavo-L1vi/[email protected]>
---
 drivers/infiniband/core/nldev.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)
It will be much better to fix the tool instead of fixing ghost case. This scenario is impossible for all those flows.
We can receive the skv/msg in two ways:
* First by allocating new message with NLMSG_DEFAULT_SIZE, which has
more room
than nlmsg_total_size(payload), payload is 0.
* Second by getting from netlink.c and it will be at least "struct
nlmsghdr" too.
Quoted 2 lines
Can you please add this info to the commit message?
Actually, I was planing to send a new version of this patch. This time using the unlikely macro for all the null checks on nlh.
Quoted 7 lines
Quoted 1 line
What do you think?
It is not datapath, so "unlikely" is not needed. Let's assume that smart enough
compiler will optimize such flow anyway, because nlmsg_put returns NULL in unlikely scenario, so this check will be unlikely automatically too.

Quoted 5 lines

I'm curious about why the return value of nlmsg_put is null checked 118 out of 129 times (based on Coverity reports) in the last linux-next tree.
So based on what you mention, do you think all those checks are actually unnecessary and, maybe they should be removed?

I honestly don't know about all cases, but if message is allocated with NLMSG_DEFAULT_SIZE and payload is 0, this check won't be needed.

So go ahead, add check if (!...) in all places, but be cautious with "potential null dereference" claims, it is not always true.

Thanks

Quoted 10 lines

Thanks
--
Gustavo

Gustavo A. R. Silva

2018-02-09 17:36 UTC #6 lore raw

Quoting Leon Romanovsky <[email protected]>:

Quoted 61 lines · Gustavo A. R. Silva

Quoted 60 lines
Quoting Leon Romanovsky <[email protected]>:
Quoted 19 lines · Gustavo A. R. Silva
Quoted 18 lines
Hi Leon,
Quoting Leon Romanovsky <[email protected]>:
Quoted 14 lines · Gustavo A. R. Silva
Quoted 13 lines
In case the message header and payload cannot be stored, function nlmsg_put returns null.
Fix this by adding multiple sanity checks and avoid a potential null dereference on _nlh_ when calling nlmsg_end.
Addresses-Coverity-ID: 1454215 ("Dereference null return value")
Addresses-Coverity-ID: 1454223 ("Dereference null return value")
Addresses-Coverity-ID: 1454224 ("Dereference null return value")
Addresses-Coverity-ID: 1464669 ("Dereference null return value")
Addresses-Coverity-ID: 1464670 ("Dereference null return value")
Addresses-Coverity-ID: 1464672 ("Dereference null return value")
Fixes e5c9469efcb1 ("RDMA/netlink: Add nldev device doit
Quoted 2 lines
Quoted 2 lines
implementation")
Quoted 1 line
Quoted 1 line
Fixes c3f66f7b0052 ("RDMA/netlink: Implement nldev port
doit callback")
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
Quoted 1 line
Fixes 7d02f605f0dc ("RDMA/netlink: Add nldev port dumpit
implementation")
Quoted 2 lines
Quoted 2 lines
Fixes b5fa635aab8f ("RDMA/nldev: Provide detailed QP information")
Fixes bf3c5a93c523 ("RDMA/nldev: Provide global resource
utilization")
Quoted 15 lines
Quoted 15 lines
Quoted 15 lines
Quoted 5 lines
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/infiniband/core/nldev.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)
It will be much better to fix the tool instead of fixing ghost case. This scenario is impossible for all those flows.
We can receive the skv/msg in two ways:
* First by allocating new message with NLMSG_DEFAULT_SIZE, which has
more room
than nlmsg_total_size(payload), payload is 0.
* Second by getting from netlink.c and it will be at least "struct
nlmsghdr" too.
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
Can you please add this info to the commit message?
Actually, I was planing to send a new version of this patch.
This time using
Quoted 9 lines
Quoted 3 lines
the unlikely macro for all the null checks on nlh.
What do you think?
It is not datapath, so "unlikely" is not needed. Let's assume that smart enough
compiler will optimize such flow anyway, because nlmsg_put returns NULL in unlikely scenario, so this check will be unlikely automatically too.

Quoted 9 lines

Quoted 5 lines
I'm curious about why the return value of nlmsg_put is null checked 118 out of 129 times (based on Coverity reports) in the last linux-next tree.
So based on what you mention, do you think all those checks are actually unnecessary and, maybe they should be removed?
I honestly don't know about all cases, but if message is allocated with NLMSG_DEFAULT_SIZE and payload is 0, this check won't be needed.

I got it.

Quoted 3 lines

So go ahead, add check if (!...) in all places, but be cautious with "potential null dereference" claims, it is not always true.

You are right. I will update the subject and commit message.

Thanks for the feedback, Leon.
I appreciate it.
--
Gustavo

Gustavo A. R. Silva

2018-02-12 22:30 UTC #7 lore raw

Hi Leon,

Quoted 76 lines · Gustavo A. R. Silva

Quoting Leon Romanovsky <[email protected]>:
Quoted 73 lines · Gustavo A. R. Silva
Quoted 72 lines
Quoting Leon Romanovsky <[email protected]>:
Quoted 6 lines · Gustavo A. R. Silva
Quoted 5 lines
Hi Leon,
Quoting Leon Romanovsky <[email protected]>:
Quoted 1 line
On Fri, Feb 09, 2018 at 12:37:02AM -0600, Gustavo A. R. Silva
Quoted 1 line · wrote:
Quoted 1 line
Quoted 1 line
Quoted 1 line
In case the message header and payload cannot be stored,
function
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
nlmsg_put returns null.
Fix this by adding multiple sanity checks and avoid a potential null dereference on _nlh_ when calling nlmsg_end.
Addresses-Coverity-ID: 1454215 ("Dereference null return value")
Addresses-Coverity-ID: 1454223 ("Dereference null return value")
Addresses-Coverity-ID: 1454224 ("Dereference null return value")
Addresses-Coverity-ID: 1464669 ("Dereference null return value")
Addresses-Coverity-ID: 1464670 ("Dereference null return value")
Addresses-Coverity-ID: 1464672 ("Dereference null return value")
Fixes e5c9469efcb1 ("RDMA/netlink: Add nldev device doit
Quoted 2 lines
Quoted 2 lines
implementation")
Quoted 1 line
Quoted 1 line
Fixes c3f66f7b0052 ("RDMA/netlink: Implement nldev port doit
callback")
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
Quoted 1 line
Fixes 7d02f605f0dc ("RDMA/netlink: Add nldev port dumpit
implementation")
Quoted 1 line
Quoted 1 line
Fixes b5fa635aab8f ("RDMA/nldev: Provide detailed QP
information")
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Fixes bf3c5a93c523 ("RDMA/nldev: Provide global resource
utilization")
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
drivers/infiniband/core/nldev.c | 26 +++++++++++++++++++++++++-
  1 file changed, 25 insertions(+), 1 deletion(-)
It will be much better to fix the tool instead of fixing ghost
case.
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
This scenario is impossible for all those flows. We can receive the skv/msg in two ways:
* First by allocating new message with NLMSG_DEFAULT_SIZE,
which has
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
more room
than nlmsg_total_size(payload), payload is 0.
* Second by getting from netlink.c and it will be at least
"struct
Quoted 6 lines
Quoted 6 lines
Quoted 4 lines
nlmsghdr" too.
Can you please add this info to the commit message?
Actually, I was planing to send a new version of this patch. This
time using
Quoted 5 lines
Quoted 3 lines
the unlikely macro for all the null checks on nlh.
What do you think?
It is not datapath, so "unlikely" is not needed. Let's assume that
smart
Quoted 2 lines
enough
compiler will optimize such flow anyway, because nlmsg_put returns
NULL
Quoted 1 line
in unlikely scenario, so this check will be unlikely automatically
too.
Quoted 1 line
I'm curious about why the return value of nlmsg_put is null checked 118 out of 129 times (based on Coverity reports) in the last linux-next tree.

Quoted 13 lines

Quoted 6 lines
Quoted 2 lines
So based on what you mention, do you think all those checks are actually unnecessary and, maybe they should be removed?
I honestly don't know about all cases, but if message is allocated with NLMSG_DEFAULT_SIZE and payload is 0, this check won't be needed.
I got it.
Quoted 3 lines
So go ahead, add check if (!...) in all places, but be cautious with "potential null dereference" claims, it is not always true.

I've finally decided to document all these cases as False Positives in the Coverity platform.

I think it is better to do that than adding unnecessary code. I will also add a link to this conversation to the Coverity database.

Thanks a lot for your feedback.
--
Gustavo