2021-12-10 07:02:27

by Yin Xiujiang

Subject: [PATCH] s390/3215: fix the array may be out of bounds

if the variable 'line' is NR_3215,
the 'raw3215[line]' will be invalid

Signed-off-by: Yin Xiujiang <[email protected]>
---
drivers/s390/char/con3215.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/s390/char/con3215.c b/drivers/s390/char/con3215.c
index f356607835d8..29409d4ca4d5 100644
--- a/drivers/s390/char/con3215.c
+++ b/drivers/s390/char/con3215.c
@@ -687,7 +687,8 @@ static void raw3215_remove (struct ccw_device *cdev)
for (line = 0; line < NR_3215; line++)
if (raw3215[line] == raw)
break;
- raw3215[line] = NULL;
+ if (line < NR_3215)
+ raw3215[line] = NULL;
spin_unlock(&raw3215_device_lock);
dev_set_drvdata(&cdev->dev, NULL);
raw3215_free_info(raw);
--
2.30.0

2021-12-10 16:05:20

by Heiko Carstens

Subject: Re: [PATCH] s390/3215: fix the array may be out of bounds

On Fri, Dec 10, 2021 at 03:02:17PM +0800, Yin Xiujiang wrote:
> if the variable 'line' is NR_3215,
> the 'raw3215[line]' will be invalid
>
> Signed-off-by: Yin Xiujiang <[email protected]>
> ---
> drivers/s390/char/con3215.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/s390/char/con3215.c b/drivers/s390/char/con3215.c
> index f356607835d8..29409d4ca4d5 100644
> --- a/drivers/s390/char/con3215.c
> +++ b/drivers/s390/char/con3215.c
> @@ -687,7 +687,8 @@ static void raw3215_remove (struct ccw_device *cdev)
> for (line = 0; line < NR_3215; line++)
> if (raw3215[line] == raw)
> break;
> - raw3215[line] = NULL;
> + if (line < NR_3215)
> + raw3215[line] = NULL;

This doesn't make sense to me. This could only happen if a device that
was never probed would be removed. The original code could have been
written better to make that more obvious, but with this patch the code
will become even more confusing.

Therefore not applying.