linux-kernel - Re: [PATCH] paride/pcd: return earlier when an error happens in pcd

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAKwvOdk=3GrWN_h41nPi_TKgw1uwB2twaV38B+Fj_paVuZ2nxw@mail.gmail.com>
Date:   Tue, 17 Jan 2023 11:21:55 -0800
From:   Nick Desaulniers <ndesaulniers@...gle.com>
To:     Tom Rix <trix@...hat.com>
Cc:     tim@...erelk.net, axboe@...nel.dk, nathan@...nel.org,
        linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
        llvm@...ts.linux.dev
Subject: Re: [PATCH] paride/pcd: return earlier when an error happens in pcd_atapi()

On Sat, Jan 14, 2023 at 9:01 AM Tom Rix <trix@...hat.com> 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;
>                         ~~~~~~~~~ ^
>
> When the call to pcd_atapi() fails, buffer[] is in an unknown state,
> so return early.
>
> Signed-off-by: Tom Rix <trix@...hat.com>
> ---
>  drivers/block/paride/pcd.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/block/paride/pcd.c b/drivers/block/paride/pcd.c
> index a5ab40784119..4524d8880463 100644
> --- a/drivers/block/paride/pcd.c
> +++ b/drivers/block/paride/pcd.c
> @@ -827,12 +827,13 @@ static int pcd_audio_ioctl(struct cdrom_device_info *cdi, unsigned int cmd, void
>                         char buffer[32];
>                         int r;

Hi Tom, Thanks for the patch!
It looks like `r` is now unused; mind removing that in v2?

Same below.

>
> -                       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 +846,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 +867,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.27.0
>
>


-- 
Thanks,
~Nick Desaulniers