[RFC,v1,1/2] vfio-ccw: Don't assume there are more ccws after a TIC
diff mbox series

Message ID d63748c1f1b03147bcbf401596638627a5e35ef7.1548082107.git.alifm@linux.ibm.com
State New
Headers show
Series
  • Small vfio-ccw fixes
Related show

Commit Message

Farhan Ali Jan. 21, 2019, 2:54 p.m. UTC
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(-)

Comments

Cornelia Huck Jan. 21, 2019, 5:38 p.m. UTC | #1
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.
Halil Pasic Jan. 21, 2019, 5:58 p.m. UTC | #2
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++;
Cornelia Huck Jan. 22, 2019, 10:12 a.m. UTC | #3
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.

Patch
diff mbox series

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++;