Patchwork Check the value of doi before referencing it

login
register
mail settings
Submitter Huzaifa Sidhpurwala
Date May 18, 2011, 4:29 a.m.
Message ID <1305692980-4730-1-git-send-email-huzaifas@redhat.com>
Download mbox | patch
Permalink /patch/96119/
State Rejected
Delegated to: David Miller
Headers show

Comments

Huzaifa Sidhpurwala - May 18, 2011, 4:29 a.m.
Value of doi is not checked before referencing it.
Though this does not cause any null pointer dereference since
all the callers of cipso_v4_doi_add check the value of doi
before calling the function, but it would be a good programming
practice to do so anyways :)

Signed-off-by: Huzaifa Sidhpurwala <huzaifas@redhat.com>
---
 net/ipv4/cipso_ipv4.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)
David Miller - May 18, 2011, 5:04 a.m.
From: Huzaifa Sidhpurwala <huzaifas@redhat.com>
Date: Wed, 18 May 2011 09:59:40 +0530

> Value of doi is not checked before referencing it.
> Though this does not cause any null pointer dereference since
> all the callers of cipso_v4_doi_add check the value of doi
> before calling the function, but it would be a good programming
> practice to do so anyways :)
> 
> Signed-off-by: Huzaifa Sidhpurwala <huzaifas@redhat.com>

I don't think we should fix bugs that do not exist.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Moore - May 18, 2011, 10:32 p.m.
On Wednesday, May 18, 2011 1:04:53 AM David Miller wrote:
> From: Huzaifa Sidhpurwala <huzaifas@redhat.com>
> Date: Wed, 18 May 2011 09:59:40 +0530
> 
> > Value of doi is not checked before referencing it.
> > Though this does not cause any null pointer dereference since
> > all the callers of cipso_v4_doi_add check the value of doi
> > before calling the function, but it would be a good programming
> > practice to do so anyways :)
> > 
> > Signed-off-by: Huzaifa Sidhpurwala <huzaifas@redhat.com>
> 
> I don't think we should fix bugs that do not exist.

I agree with David.

If there were a large number of callers or cipso_v4_doi_add() was a more 
general function there might be some merit in performing more sanity checks on 
the values passed to the function.  However, as it stands, cipso_v4_doi_add() 
is a fairly specialized function which is called by a small number of 
functions all of which are internal to NetLabel.

--
paul moore
linux @ hp
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
index a0af7ea..7adc4ea 100644
--- a/net/ipv4/cipso_ipv4.c
+++ b/net/ipv4/cipso_ipv4.c
@@ -473,10 +473,13 @@  int cipso_v4_doi_add(struct cipso_v4_doi *doi_def,
 	u32 doi_type;
 	struct audit_buffer *audit_buf;
 
-	doi = doi_def->doi;
-	doi_type = doi_def->type;
+	if (doi_def) {
+		doi = doi_def->doi;
+		doi_type = doi_def->type;
+	} else
+	goto doi_add_return;
 
-	if (doi_def == NULL || doi_def->doi == CIPSO_V4_DOI_UNKNOWN)
+	if (doi_def->doi == CIPSO_V4_DOI_UNKNOWN)
 		goto doi_add_return;
 	for (iter = 0; iter < CIPSO_V4_TAG_MAXCNT; iter++) {
 		switch (doi_def->tags[iter]) {