media: tuners: tda18271: fix error code handling in tda18271_attach()
Subsystems (2)
PATCH 1/3 media: tuners: tda18271: fix error code handling in tda18271_attach()
tda18271_attach() uses the hybrid_tuner_request_state() macro. It may return the error code -ENOMEM, but the function handle the value 0 instead.
Found by Linux Verification Center (linuxtesting.org) with Svace.
--- drivers/media/tuners/tda18271-fe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/media/tuners/tda18271-fe.c +1 -1
@@ -1255,7 +1255,7 @@ struct dvb_frontend *tda18271_attach(struct dvb_frontend *fe, u8 addr, hybrid_tuner_instance_list, i2c, addr, "tda18271");switch (instance) { -case 0 : +case -ENOMEM : goto fail;case 1 :/* new tuner instance */
Signature · 1 line
2.34.1
PATCH 2/3 media: tuners: xc5000: fix error code handling in xc5000_attach()
xc5000_attach() uses the hybrid_tuner_request_state() macro. It may return the error code -ENOMEM, but the function handle the value 0 instead.
Found by Linux Verification Center (linuxtesting.org) with Svace.
--- drivers/media/tuners/xc5000.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/media/tuners/xc5000.c +1 -1
@@ -1379,7 +1379,7 @@ struct dvb_frontend *xc5000_attach(struct dvb_frontend *fe, hybrid_tuner_instance_list, i2c, cfg->i2c_address, "xc5000");switch (instance) { -case 0 : +case -ENOMEM : goto fail;case 1 :/* new tuner instance */
Signature · 1 line
2.34.1
Re: [PATCH 2/3] media: tuners: xc5000: fix error code handling in xc5000_attach()
Quoted 2 lines · Roman Smirnov
xc5000_attach()uses thehybrid_tuner_request_state()macro. It may return the error code-ENOMEM, but the function handle
Handles (just noticed that grammar issue in all 3 patches).
Quoted 6 lines
the value 0 instead.
Found by Linux Verification Center (linuxtesting.org) with Svace.
Fixes b9302fa7ed97 ("media: tuners: fix error return code of hybrid_tuner_request_state()")Signed-off-by Roman Smirnov <[email protected]>
[...]
MBR, Sergey
PATCH 3/3 media: tuners: tuner_simple: fix error code handling in simple_tuner_attach()
In reply to Roman Smirnovsimple_tuner_attach() uses the hybrid_tuner_request_state() macro. It may return the error code -ENOMEM, but the function handle the value 0 instead.
Found by Linux Verification Center (linuxtesting.org) with Svace.
--- drivers/media/tuners/tuner-simple.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/media/tuners/tuner-simple.c +1 -1
@@ -1089,7 +1089,7 @@ struct dvb_frontend *simple_tuner_attach(struct dvb_frontend *fe, i2c_adap, i2c_addr, "tuner-simple");switch (instance) { -case 0 : +case -ENOMEM :mutex_unlock (&tuner_simple_list_mutex);return NULL ;case 1 :
Signature · 1 line
2.34.1
Re: [PATCH 3/3] media: tuners: tuner_simple: fix error code handling in simple_tuner_attach()
Hello!
Should have been "tuner-simple:" in the subject... :-)
Quoted 8 lines · Roman Smirnov
simple_tuner_attach()uses thehybrid_tuner_request_state()macro. It may return the error code-ENOMEM, but the function handle the value 0 instead.Found by Linux Verification Center (linuxtesting.org) with Svace.
Fixes b9302fa7ed97 ("media: tuners: fix error return code of hybrid_tuner_request_state()")Signed-off-by Roman Smirnov <[email protected]>
[...]
MBR, Sergey
Quoted 8 lines · Roman Smirnov
tda18271_attach()uses thehybrid_tuner_request_state()macro. It may return the error code-ENOMEM, but the function handle the value 0 instead.Found by Linux Verification Center (linuxtesting.org) with Svace.
Fixes b9302fa7ed97 ("media: tuners: fix error return code of hybrid_tuner_request_state()")Signed-off-by Roman Smirnov <[email protected]>
[...]
MBR, Sergey
Hello Roman,
Quoted 3 lines · Roman Smirnov
tda18271_attach()uses thehybrid_tuner_request_state()macro. It may return the error code-ENOMEM, but the function handle the value 0 instead.
Maybe hybrid_tuner_request_state macro declaration should be fixed to generate zero in case of a memory allocation failure?
At least it has a comment stating the following
* 0 - no instances, indicates an error - kzalloc must have failed
And supposedly a number of drivers implemented the error handling based on this assumption.
The drivers mentioned in this series are not the only ones susceptible to the problem. Grepping through "hybrid_tuner_request_state" calls also gives out tda9887, xc2028, r820t and others.
Quoted 25 lines
Found by Linux Verification Center (linuxtesting.org) with Svace.
Fixes b9302fa7ed97 ("media: tuners: fix error return code of hybrid_tuner_request_state()")Signed-off-by Roman Smirnov <[email protected]>--- drivers/media/tuners/tda18271-fe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/media/tuners/tda18271-fe.c +1
-1@@ -1255,7 +1255,7 @@ struct dvb_frontend *tda18271_attach(struct dvb_frontend *fe, u8 addr, hybrid_tuner_instance_list, i2c, addr, "tda18271");switch (instance) { -case 0 : +case -ENOMEM : goto fail;case 1 :/* new tuner instance */ Signature · 1 line
2.34.1
Quoted 13 lines · Fedor Pchelkin
Hello Roman,
Quoted 3 lines · Roman Smirnov
tda18271_attach()uses thehybrid_tuner_request_state()macro. It may return the error code-ENOMEM, but the function handle the value 0 instead.Maybe
hybrid_tuner_request_statemacro declaration should be fixed to generate zero in case of a memory allocation failure?At least it has a comment stating the following
* 0 - no instances, indicates an error - kzalloc must have failed
Quoted 11 lines
And supposedly a number of drivers implemented the error handling based on this assumption.
The drivers mentioned in this series are not the only ones susceptible to the problem. Grepping through "
hybrid_tuner_request_state" calls also gives out tda9887, xc2028, r820t and others.Quoted 4 lines
Found by Linux Verification Center (linuxtesting.org) with Svace.
Fixes b9302fa7ed97 ("media: tuners: fix error return code of hybrid_tuner_request_state()")
Looking more thoroughly, I think commit b9302fa7ed97 ("media: tuners: fix error return code of hybrid_tuner_request_state()") should be reverted because it just contradicts with the return values contract which is stated in the comment for the macro and which is followed by all the existing drivers.
__ret should be assigned 0 in error case as was before the commit.
Quoted 21 lines
Quoted 21 lines
Signed-off-by Roman Smirnov <[email protected]>--- drivers/media/tuners/tda18271-fe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/media/tuners/tda18271-fe.c +1
-1@@ -1255,7 +1255,7 @@ struct dvb_frontend *tda18271_attach(struct dvb_frontend *fe, u8 addr, hybrid_tuner_instance_list, i2c, addr, "tda18271");switch (instance) { -case 0 : +case -ENOMEM : goto fail;case 1 :/* new tuner instance */ Signature · 1 line
2.34.1