Skip to contentSkip to conversation

dvb_frontend_ioctl(): fix check-after-use

PATCH In review Adrian Bunk 4 participants 5 messages Jul 30, 2007 – Aug 11, 2007 Media (V4L/DVB)

lore
Share & tools
lore

Adrian Bunk

author
#0 lore raw

PATCH 1/1 dvb_frontend_ioctl(): fix check-after-use

The Coverity checker spotted that we have already oops'ed if "fe" was NULL.

Since "fe" being NULL seems impossible at this point this patch removes the NULL check.

Signed-off-by Adrian Bunk <[email protected]>

---

drivers/media/dvb/dvb-core/dvb_frontend.c renamed from drivers/media/dvb/dvb-core/dvb_frontend.c.old +1 -1
@@ -706,11 +706,11 @@ static int dvb_frontend_ioctl(struct ino
 	struct dvb_frontend_private *fepriv = fe->frontend_priv;
 	int err = -EOPNOTSUPP;
 
 	dprintk ("%s\n", __FUNCTION__);
 
-	if (!fe || fepriv->exit)
+	if (fepriv->exit)
 		return -ENODEV;
 
 	if ((file->f_flags & O_ACCMODE) == O_RDONLY &&
 	    (_IOC_DIR(cmd) != _IOC_READ || cmd == FE_GET_EVENT ||
 	     cmd == FE_DISEQC_RECV_SLAVE_REPLY))
Acked-by on v1 1/1
#1 lore raw
Quoted 15 lines · Adrian Bunk

The Coverity checker spotted that we have already oops'ed if "fe" was NULL.

Since "fe" being NULL seems impossible at this point this patch removes the NULL check.

Signed-off-by Adrian Bunk <[email protected]>

---

drivers/media/dvb/dvb-core/dvb_frontend.c 2007-07-29 21:42:16.000000000 +0200 renamed from drivers/media/dvb/dvb-core/dvb_frontend.c.old 2007-07-29 21:41:56.000000000 +0200 +0 -0
@@ -706,11 +706,11 @@ static int dvb_frontend_ioctl(struct ino
        struct dvb_frontend_private *fepriv = fe->frontend_priv;
        int err = -EOPNOTSUPP;
 
        dprintk ("%s\n", __FUNCTION__);

drivers/media/dvb/dvb-core/dvb_frontend.c 2007-07-29 21:42:16.000000000 +0200 static int dvb_frontend_ioctl(struct ino

Quoted 10 lines
drivers/media/dvb/dvb-core/dvb_frontend.c 2007-07-29 21:42:16.000000000 +0200 renamed from drivers/media/dvb/dvb-core/dvb_frontend.c.old 2007-07-29 21:41:56.000000000 +0200 +1 -2
 
-       if (!fe || fepriv->exit)
+       if (fepriv->exit)
                return -ENODEV;
 
        if ((file->f_flags & O_ACCMODE) == O_RDONLY &&
            (_IOC_DIR(cmd) != _IOC_READ || cmd == FE_GET_EVENT ||
             cmd == FE_DISEQC_RECV_SLAVE_REPLY))
 
-
Acked-by Manu Abraham <[email protected]>
#2 lore raw
Quoted 24 lines · Manu Abraham
Quoted 1 line · Adrian Bunk

The Coverity checker spotted that we have already oops'ed if "fe" was

NULL.

Quoted 8 lines

Since "fe" being NULL seems impossible at this point this patch removes the NULL check.

Signed-off-by Adrian Bunk <[email protected]>

---
--- linux-2.6.23-rc1-mm1/drivers/media/dvb/dvb-core/dvb_frontend.c.old

2007-07-29 21:41:56.000000000 +0200
Quoted 1 line

+++ linux-2.6.23-rc1-mm1/drivers/media/dvb/dvb-core/dvb_frontend.c

2007-07-29 21:42:16.000000000 +0200
Quoted 10 lines
drivers/media/dvb/dvb-core/dvb_frontend.c renamed from drivers/media/dvb/dvb-core/dvb_frontend.c.old +1 -1
@@ -706,11 +706,11 @@ static int dvb_frontend_ioctl(struct ino
        struct dvb_frontend_private *fepriv = fe->frontend_priv;
        int err = -EOPNOTSUPP;
 
        dprintk ("%s\n", __FUNCTION__);
 
-       if (!fe || fepriv->exit)
+       if (fepriv->exit)
                return -ENODEV;
Quoted 8 lines

drivers/media/dvb/dvb-core/dvb_frontend.c static int dvb_frontend_ioctl(struct ino

Quoted 5 lines
drivers/media/dvb/dvb-core/dvb_frontend.c renamed from drivers/media/dvb/dvb-core/dvb_frontend.c.old +0 -1
        if ((file->f_flags & O_ACCMODE) == O_RDONLY &&
            (_IOC_DIR(cmd) != _IOC_READ || cmd == FE_GET_EVENT ||
             cmd == FE_DISEQC_RECV_SLAVE_REPLY))
 
-
Acked-by Manu Abraham <[email protected]>

This issue has been known for a while including some other problems at that part.

http://article.gmane.org/gmane.linux.drivers.dvb/35351/match=patch+dvb_net+hotplugging+support

this includes a link where this and more got discussed in May.

Markus

#3 lore raw
Quoted 15 lines · Markus Rechberger
Quoted 10 lines · Manu Abraham
Quoted 1 line · Adrian Bunk

The Coverity checker spotted that we have already oops'ed if "fe" was

NULL.

drivers/media/dvb/dvb-core/dvb_frontend.c static int dvb_frontend_ioctl(struct ino · line 707

Quoted 7 lines
drivers/media/dvb/dvb-core/dvb_frontend.c renamed from drivers/media/dvb/dvb-core/dvb_frontend.c.old +1 -1
@@ -706,11 +706,11 @@ static int dvb_frontend_ioctl(struct ino
-       if (!fe || fepriv->exit)
+       if (fepriv->exit)
                return -ENODEV;

This issue has been known for a while including some other problems at that part.

Quoted 3 lines

http://article.gmane.org/gmane.linux.drivers.dvb/35351/match=patch+dvb_net+hotplugging+support

this includes a link where this and more got discussed in May.

For dvb_net_close, I like the patch I already posted better. To fix the check-after-use, it's not "use" part that's the problem, it's the "check" part that isn't necessary.

I traced the dvb-net code, http://article.gmane.org/gmane.linux.kernel/543689,
and I'm sure that dvbdev can't be NULL.

My patch also deletes a few pieces of duplicated code by calling dvb_generic_release().

The only problem is that practically no one uses dvb-net, so it's very hard to test these patches.

In all the dvb code, were is the locking for device open and release? I don't see it. What is preventing two threads from trying to open and/or close the same dvb device at the same time?

#4 lore raw
Quoted 30 lines · Trent Piepho
Quoted 5 lines · Markus Rechberger
Quoted 4 lines · Manu Abraham
Quoted 1 line · Adrian Bunk

The Coverity checker spotted that we have already oops'ed if "fe" was

NULL.

Quoted 1 line
Quoted 11 lines
Quoted 6 lines

drivers/media/dvb/dvb-core/dvb_frontend.c static int dvb_frontend_ioctl(struct ino · line 707

Quoted 6 lines
drivers/media/dvb/dvb-core/dvb_frontend.c renamed from drivers/media/dvb/dvb-core/dvb_frontend.c.old +1 -1
@@ -706,11 +706,11 @@ static int dvb_frontend_ioctl(struct ino
-       if (!fe || fepriv->exit)
+       if (fepriv->exit)
                return -ENODEV;

This issue has been known for a while including some other problems at that part.

http://article.gmane.org/gmane.linux.drivers.dvb/35351/match=patch+dvb_net+hotplugging+support

Quoted 2 lines

this includes a link where this and more got discussed in May.

For dvb_net_close, I like the patch I already posted better. To fix the check-after-use, it's not "use" part that's the problem, it's the "check" part
that isn't necessary.

I traced the dvb-net code,
http://article.gmane.org/gmane.linux.kernel/543689,
and I'm sure that dvbdev can't be NULL.

Quoted 7 lines

My patch also deletes a few pieces of duplicated code by calling dvb_generic_release().

The only problem is that practically no one uses dvb-net, so it's very hard to
test these patches.

I didn't see this patch, it would be fine if it would get applied.

Quoted 5 lines

In all the dvb code, were is the locking for device open and release? I don't
see it. What is preventing two threads from trying to open and/or close the same dvb device at the same time?

Haven't looked at that yet, since I moreover work to get the em28xx driver included now.

Markus