| Message ID | d63748c1f1b03147bcbf401596638627a5e35ef7.1548082107.git.alifm@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Small vfio-ccw fixes | expand |
On Mon, 21 Jan 2019 09:54:08 -0500 Farhan Ali <alifm@linux.ibm.com> wrote: > When trying to calculate the length of a ccw chain, we assume > there are ccws after a TIC. This can lead to overcounting and > copying garbage data from guest memory. > > Signed-off-by: Farhan Ali <alifm@linux.ibm.com> > --- > drivers/s390/cio/vfio_ccw_cp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c > index 17a1ee3..a820a21 100644 > --- a/drivers/s390/cio/vfio_ccw_cp.c > +++ b/drivers/s390/cio/vfio_ccw_cp.c > @@ -392,7 +392,7 @@ static int ccwchain_calc_length(u64 iova, struct channel_program *cp) > return -EOPNOTSUPP; > } > > - if ((!ccw_is_chain(ccw)) && (!ccw_is_tic(ccw))) > + if (!ccw_is_chain(ccw)) > break; > > ccw++; This looks like the right thing to do.
On Mon, 21 Jan 2019 09:54:08 -0500 Farhan Ali <alifm@linux.ibm.com> wrote: > When trying to calculate the length of a ccw chain, we assume > there are ccws after a TIC. This can lead to overcounting and > copying garbage data from guest memory. > I wonder what was the idea behind this. With ccw one can implement branching using TIC and status modifier, so that the next ccw is either the one designated by the TIC or the one after the TIC. But from the perspective the status modifier mechanism TIC ain't special (except that its execution won't present the status modifier bit). Basically we never know if the end of the chain is really the end of the chain. We could kind of handle that either by doing some sort of poisoning or by some sort of suspend-resume magic (which can not be perfect, because resume starts a new command chain as observed by the device). > Signed-off-by: Farhan Ali <alifm@linux.ibm.com> Reviewed-by: Halil Pasic <pasic@linux.ibm.com> > --- > drivers/s390/cio/vfio_ccw_cp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c > index 17a1ee3..a820a21 100644 > --- a/drivers/s390/cio/vfio_ccw_cp.c > +++ b/drivers/s390/cio/vfio_ccw_cp.c > @@ -392,7 +392,7 @@ static int ccwchain_calc_length(u64 iova, struct channel_program *cp) > return -EOPNOTSUPP; > } > > - if ((!ccw_is_chain(ccw)) && (!ccw_is_tic(ccw))) > + if (!ccw_is_chain(ccw)) > break; > > ccw++;
On Mon, 21 Jan 2019 09:54:08 -0500 Farhan Ali <alifm@linux.ibm.com> wrote: > When trying to calculate the length of a ccw chain, we assume > there are ccws after a TIC. This can lead to overcounting and > copying garbage data from guest memory. > > Signed-off-by: Farhan Ali <alifm@linux.ibm.com> > --- > drivers/s390/cio/vfio_ccw_cp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c > index 17a1ee3..a820a21 100644 > --- a/drivers/s390/cio/vfio_ccw_cp.c > +++ b/drivers/s390/cio/vfio_ccw_cp.c > @@ -392,7 +392,7 @@ static int ccwchain_calc_length(u64 iova, struct channel_program *cp) > return -EOPNOTSUPP; > } > > - if ((!ccw_is_chain(ccw)) && (!ccw_is_tic(ccw))) > + if (!ccw_is_chain(ccw)) > break; > > ccw++; Thanks, applied.
diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c index 17a1ee3..a820a21 100644 --- a/drivers/s390/cio/vfio_ccw_cp.c +++ b/drivers/s390/cio/vfio_ccw_cp.c @@ -392,7 +392,7 @@ static int ccwchain_calc_length(u64 iova, struct channel_program *cp) return -EOPNOTSUPP; } - if ((!ccw_is_chain(ccw)) && (!ccw_is_tic(ccw))) + if (!ccw_is_chain(ccw)) break; ccw++;
When trying to calculate the length of a ccw chain, we assume there are ccws after a TIC. This can lead to overcounting and copying garbage data from guest memory. Signed-off-by: Farhan Ali <alifm@linux.ibm.com> --- drivers/s390/cio/vfio_ccw_cp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)