(Apologies for the delay; I was on holiday when this came in and it was lost in the noise.) On Tue, 2022-07-12 at 17:52 +0800, sohu0106 wrote: > > > > In the vfio_ccw_async_region_write/vfio_ccw_async_region_read > > > function of drivers/s390/cio/vfio_ccw_async.c, parameter "size_t > > > count" is pass by userland, if "count" is very large, it will > > bypass > "if (pos + count > sizeof(*region))".(such as > > count=0xffffffff). Then > it will lead to buffer overflow in > > "copy_from_user((void *)region + > pos, buf, count)". There are some mechanical problems with this patch. It needs a Signed- off-by tag, and is not applicable in its current form. All easy to resolve, and documented in: Documentation/process/submitting-patches.rst Having said that, I'm not sure that the problem exists as you describe. In my tests, submitting a very large count (0xffffffff, per your example) gets capped to MAX_RW_COUNT [1] before it gets here. If you have a mechanism that shows otherwise, I'd like to see it so that I can revisit my own test scenarios. Seeing it fail would imply that the async region is not the only one affected, and a new version of this patch would need to also address the io and schib regions in vfio-ccw. Thanks, Eric [1] https://lore.kernel.org/all/CAADWXX9rrESSEGmA4C9F85E9jo7H-pv+CUtyAU_kyB=DfcHRpA@xxxxxxxxxxxxxx/ > > > > > > > > > > diff --git a/vfio_ccw_async.c_org b/vfio_ccw_async.c > > index 7a838e3..33339ad 100644 > > --- a/vfio_ccw_async.c_org > > +++ b/vfio_ccw_async.c > > @@ -21,7 +21,7 @@ static ssize_t vfio_ccw_async_region_read(struct > > > vfio_ccw_private *private, > > struct ccw_cmd_region *region; > > int ret; > > > > > > - if (pos + count > sizeof(*region)) > > + if (pos + count > sizeof(*region) && pos + count > pos) > > return -EINVAL; > > > > > > mutex_lock(&private->io_mutex); > > @@ -43,7 +43,7 @@ static ssize_t vfio_ccw_async_region_write(struct > > > vfio_ccw_private *private, > > struct ccw_cmd_region *region; > > int ret; > > > > > > - if (pos + count > sizeof(*region)) > > + if (pos + count > sizeof(*region) && pos + count > pos) > > return -EINVAL; > > > > > > if (!mutex_trylock(&private->io_mutex)) > >
