[PATCH v2] paride/pcd: return earlier when an error happens in pcd_atapi()

[PATCH v2] paride/pcd: return earlier when an error happens in pcd_atapi()

[Tom Rix]

clang static analysis reports
drivers/block/paride/pcd.c:856:36: warning: The left operand of '&'
  is a garbage value [core.UndefinedBinaryOperatorResult]
  tocentry->cdte_ctrl = buffer[5] & 0xf;
                        ~~~~~~~~~ ^

When the call to pcd_atapi() fails, buffer[] is in an unknown state,
so return early.

Signed-off-by: Tom Rix <trix@redhat.com>
---
v2: remove unused 'r' variable
---
 drivers/block/paride/pcd.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/block/paride/pcd.c b/drivers/block/paride/pcd.c
index a5ab40784119..47757ba1a09f 100644
--- a/drivers/block/paride/pcd.c
+++ b/drivers/block/paride/pcd.c
@@ -825,14 +825,14 @@ static int pcd_audio_ioctl(struct cdrom_device_info *cdi, unsigned int cmd, void
 			struct cdrom_tochdr *tochdr =
 			    (struct cdrom_tochdr *) arg;
 			char buffer[32];
-			int r;
 
-			r = pcd_atapi(cd, cmd, 12, buffer, "read toc header");
+			if (pcd_atapi(cd, cmd, 12, buffer, "read toc header"))
+				return -EIO;
 
 			tochdr->cdth_trk0 = buffer[2];
 			tochdr->cdth_trk1 = buffer[3];
 
-			return r ? -EIO : 0;
+			return 0;
 		}
 
 	case CDROMREADTOCENTRY:
@@ -845,13 +845,13 @@ static int pcd_audio_ioctl(struct cdrom_device_info *cdi, unsigned int cmd, void
 			struct cdrom_tocentry *tocentry =
 			    (struct cdrom_tocentry *) arg;
 			unsigned char buffer[32];
-			int r;
 
 			cmd[1] =
 			    (tocentry->cdte_format == CDROM_MSF ? 0x02 : 0);
 			cmd[6] = tocentry->cdte_track;
 
-			r = pcd_atapi(cd, cmd, 12, buffer, "read toc entry");
+			if (pcd_atapi(cd, cmd, 12, buffer, "read toc entry"))
+				return -EIO;
 
 			tocentry->cdte_ctrl = buffer[5] & 0xf;
 			tocentry->cdte_adr = buffer[5] >> 4;
@@ -866,7 +866,7 @@ static int pcd_audio_ioctl(struct cdrom_device_info *cdi, unsigned int cmd, void
 				    (((((buffer[8] << 8) + buffer[9]) << 8)
 				      + buffer[10]) << 8) + buffer[11];
 
-			return r ? -EIO : 0;
+			return 0;
 		}
 
 	default:
-- 
2.26.3

Re: [PATCH v2] paride/pcd: return earlier when an error happens in pcd_atapi()

[Jens Axboe]

On 1/22/23 8:49 AM, Tom Rix wrote:
> clang static analysis reports
> drivers/block/paride/pcd.c:856:36: warning: The left operand of '&'
>   is a garbage value [core.UndefinedBinaryOperatorResult]
>   tocentry->cdte_ctrl = buffer[5] & 0xf;
>                         ~~~~~~~~~ ^

Has this one been compiled? I'm guessing not tested...

In any case, this code is going away hopefully shortly, so let's not
bother with changes like this.

-- 
Jens Axboe

Re: [PATCH v2] paride/pcd: return earlier when an error happens in pcd_atapi()

[Tom Rix]

On 1/22/23 12:49 PM, Jens Axboe wrote:
> On 1/22/23 8:49 AM, Tom Rix wrote:
>> clang static analysis reports
>> drivers/block/paride/pcd.c:856:36: warning: The left operand of '&'
>>    is a garbage value [core.UndefinedBinaryOperatorResult]
>>    tocentry->cdte_ctrl = buffer[5] & 0xf;
>>                          ~~~~~~~~~ ^
> Has this one been compiled? I'm guessing not tested...
>
> In any case, this code is going away hopefully shortly, so let's not
> bother with changes like this.

Going away soon would be nice, this is an old problem.

I did not bother with a fixes: tag because it was is when the repo was 
created in 2005.

Tom


>

Re: [PATCH v2] paride/pcd: return earlier when an error happens in pcd_atapi()

[Al Viro]

On Sun, Jan 22, 2023 at 01:49:00PM -0700, Jens Axboe wrote:
> On 1/22/23 8:49 AM, Tom Rix wrote:
> > clang static analysis reports
> > drivers/block/paride/pcd.c:856:36: warning: The left operand of '&'
> >   is a garbage value [core.UndefinedBinaryOperatorResult]
> >   tocentry->cdte_ctrl = buffer[5] & 0xf;
> >                         ~~~~~~~~~ ^
> 
> Has this one been compiled? I'm guessing not tested...
> 
> In any case, this code is going away hopefully shortly, so let's not
> bother with changes like this.

	Look at the callers - the value left in entry is discarded if
->audio_ioctl(..., CDROMREADTOCENTRY, &entry) returns non-zero.  Sure,
it's a nasal daemon territory, but realistically it's not going to be
caught by testing.

Re: [PATCH v2] paride/pcd: return earlier when an error happens in pcd_atapi()

[Jens Axboe]

On 1/22/23 3:10 PM, Al Viro wrote:
> On Sun, Jan 22, 2023 at 01:49:00PM -0700, Jens Axboe wrote:
>> On 1/22/23 8:49 AM, Tom Rix wrote:
>>> clang static analysis reports
>>> drivers/block/paride/pcd.c:856:36: warning: The left operand of '&'
>>>   is a garbage value [core.UndefinedBinaryOperatorResult]
>>>   tocentry->cdte_ctrl = buffer[5] & 0xf;
>>>                         ~~~~~~~~~ ^
>>
>> Has this one been compiled? I'm guessing not tested...
>>
>> In any case, this code is going away hopefully shortly, so let's not
>> bother with changes like this.
> 
> 	Look at the callers - the value left in entry is discarded if
> ->audio_ioctl(..., CDROMREADTOCENTRY, &entry) returns non-zero.  Sure,
> it's a nasal daemon territory, but realistically it's not going to be
> caught by testing.

I don't expect anyone really to be able to test it, but v1 had a pretty
basic issue that would've surely triggered a compiler warning had it
been compiled.

-- 
Jens Axboe