[PATCH] erofs-utils: lib: fix global-buffer-overflow due to invalid device

[PATCH] erofs-utils: lib: fix global-buffer-overflow due to invalid device

[Gao Xiang]

Fuzzer generates an image with crafted chunks of some invalid device.
Also refine the printed message of EOD.

Closes: https://github.com/erofs/erofsnightly/actions/runs/10172576269/job/28135408276
Closes: https://github.com/erofs/erofs-utils/issues/11
Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
 lib/io.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/lib/io.c b/lib/io.c
index 6bfae69..fbeff03 100644
--- a/lib/io.c
+++ b/lib/io.c
@@ -342,6 +342,10 @@ ssize_t erofs_dev_read(struct erofs_sb_info *sbi, int device_id,
 	ssize_t read;
 
 	if (device_id) {
+		if (device_id >= sbi->nblobs) {
+			erofs_err("invalid device id %u", device_id);
+			return -EIO;
+		}
 		read = erofs_io_pread(&((struct erofs_vfile) {
 				.fd = sbi->blobfd[device_id - 1],
 			}), buf, offset, len);
@@ -352,7 +356,8 @@ ssize_t erofs_dev_read(struct erofs_sb_info *sbi, int device_id,
 	if (read < 0)
 		return read;
 	if (read < len) {
-		erofs_info("reach EOF of device, pading with zeroes");
+		erofs_info("reach EOF of device @ %llu, pading with zeroes",
+			   offset | 0ULL);
 		memset(buf + read, 0, len - read);
 	}
 	return 0;
-- 
2.43.5

Re: [PATCH] erofs-utils: lib: fix global-buffer-overflow due to invalid device

[Sandeep Dhavale via Linux-erofs]

On Thu, Aug 8, 2024 at 9:04 AM Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
>
> Fuzzer generates an image with crafted chunks of some invalid device.
> Also refine the printed message of EOD.
>
> Closes: https://github.com/erofs/erofsnightly/actions/runs/10172576269/job/28135408276
> Closes: https://github.com/erofs/erofs-utils/issues/11
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> ---
>  lib/io.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/lib/io.c b/lib/io.c
> index 6bfae69..fbeff03 100644
> --- a/lib/io.c
> +++ b/lib/io.c
> @@ -342,6 +342,10 @@ ssize_t erofs_dev_read(struct erofs_sb_info *sbi, int device_id,
>         ssize_t read;
>
>         if (device_id) {
> +               if (device_id >= sbi->nblobs) {
> +                       erofs_err("invalid device id %u", device_id);
> +                       return -EIO;
> +               }
>                 read = erofs_io_pread(&((struct erofs_vfile) {
>                                 .fd = sbi->blobfd[device_id - 1],
>                         }), buf, offset, len);
> @@ -352,7 +356,8 @@ ssize_t erofs_dev_read(struct erofs_sb_info *sbi, int device_id,
>         if (read < 0)
>                 return read;
>         if (read < len) {
> -               erofs_info("reach EOF of device, pading with zeroes");
> +               erofs_info("reach EOF of device @ %llu, pading with zeroes",
> +                          offset | 0ULL);
nit: typo carried over from previous log. s/pading/padding

>                 memset(buf + read, 0, len - read);
>         }
>         return 0;
> --
> 2.43.5
>

Reviewed-by: Sandeep Dhavale <dhavale@google.com>

Thanks,
Sandeep.

Re: [PATCH] erofs-utils: lib: fix global-buffer-overflow due to invalid device

[Gao Xiang]

Hi Sandeep,

On Thu, Aug 08, 2024 at 10:15:31AM -0700, Sandeep Dhavale via Linux-erofs wrote:
> On Thu, Aug 8, 2024 at 9:04 AM Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
> >
> > Fuzzer generates an image with crafted chunks of some invalid device.
> > Also refine the printed message of EOD.
> >
> > Closes: https://github.com/erofs/erofsnightly/actions/runs/10172576269/job/28135408276
> > Closes: https://github.com/erofs/erofs-utils/issues/11
> > Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> > ---
> >  lib/io.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/io.c b/lib/io.c
> > index 6bfae69..fbeff03 100644
> > --- a/lib/io.c
> > +++ b/lib/io.c
> > @@ -342,6 +342,10 @@ ssize_t erofs_dev_read(struct erofs_sb_info *sbi, int device_id,
> >         ssize_t read;
> >
> >         if (device_id) {
> > +               if (device_id >= sbi->nblobs) {
> > +                       erofs_err("invalid device id %u", device_id);
> > +                       return -EIO;
> > +               }
> >                 read = erofs_io_pread(&((struct erofs_vfile) {
> >                                 .fd = sbi->blobfd[device_id - 1],
> >                         }), buf, offset, len);
> > @@ -352,7 +356,8 @@ ssize_t erofs_dev_read(struct erofs_sb_info *sbi, int device_id,
> >         if (read < 0)
> >                 return read;
> >         if (read < len) {
> > -               erofs_info("reach EOF of device, pading with zeroes");
> > +               erofs_info("reach EOF of device @ %llu, pading with zeroes",
> > +                          offset | 0ULL);
> nit: typo carried over from previous log. s/pading/padding

Thanks for catching this!

> 
> >                 memset(buf + read, 0, len - read);
> >         }
> >         return 0;
> > --
> > 2.43.5
> >
> 
> Reviewed-by: Sandeep Dhavale <dhavale@google.com>

I'm about to releasing erofs-utils 1.8 today (it already takes much
long time than expected, I don't want to hold it anymore), so the
code is freezed for now.

I will tag v1.8 soon, and write an announcement mail hours later.

Thanks,
Gao Xiang

> 
> Thanks,
> Sandeep.

Re: [PATCH] erofs-utils: lib: fix global-buffer-overflow due to invalid device

[Sandeep Dhavale via Linux-erofs]

On Thu, Aug 8, 2024 at 10:45 AM Gao Xiang <xiang@kernel.org> wrote:
>
> Hi Sandeep,
>
> On Thu, Aug 08, 2024 at 10:15:31AM -0700, Sandeep Dhavale via Linux-erofs wrote:
> > On Thu, Aug 8, 2024 at 9:04 AM Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
> > >
> > > Fuzzer generates an image with crafted chunks of some invalid device.
> > > Also refine the printed message of EOD.
> > >
> > > Closes: https://github.com/erofs/erofsnightly/actions/runs/10172576269/job/28135408276
> > > Closes: https://github.com/erofs/erofs-utils/issues/11
> > > Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> > > ---
> > >  lib/io.c | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/io.c b/lib/io.c
> > > index 6bfae69..fbeff03 100644
> > > --- a/lib/io.c
> > > +++ b/lib/io.c
> > > @@ -342,6 +342,10 @@ ssize_t erofs_dev_read(struct erofs_sb_info *sbi, int device_id,
> > >         ssize_t read;
> > >
> > >         if (device_id) {
> > > +               if (device_id >= sbi->nblobs) {
> > > +                       erofs_err("invalid device id %u", device_id);
> > > +                       return -EIO;
> > > +               }
> > >                 read = erofs_io_pread(&((struct erofs_vfile) {
> > >                                 .fd = sbi->blobfd[device_id - 1],
> > >                         }), buf, offset, len);
> > > @@ -352,7 +356,8 @@ ssize_t erofs_dev_read(struct erofs_sb_info *sbi, int device_id,
> > >         if (read < 0)
> > >                 return read;
> > >         if (read < len) {
> > > -               erofs_info("reach EOF of device, pading with zeroes");
> > > +               erofs_info("reach EOF of device @ %llu, pading with zeroes",
> > > +                          offset | 0ULL);
> > nit: typo carried over from previous log. s/pading/padding
>
> Thanks for catching this!
>
> >
> > >                 memset(buf + read, 0, len - read);
> > >         }
> > >         return 0;
> > > --
> > > 2.43.5
> > >
> >
> > Reviewed-by: Sandeep Dhavale <dhavale@google.com>
>
> I'm about to releasing erofs-utils 1.8 today (it already takes much
> long time than expected, I don't want to hold it anymore), so the
> code is freezed for now.
>
Hi Gao,
No problem. Just caught my eyes, it's only cosmetic anyways.

> I will tag v1.8 soon, and write an announcement mail hours later.
>
Thank you!

Regards,
Sandeep.

> Thanks,
> Gao Xiang
>
> >
> > Thanks,
> > Sandeep.