[PATCH] tty: vcc: add check for mdesc_grab()

[PATCH] tty: vcc: add check for mdesc_grab()

[Liang He]

In vcc_probe(), we should check the return value of
mdesc_grab() as it may return NULL. While the
vio_vdev_node() has the NULL-check, but if there
is still a call to mdesc_release() which may cause
a NPD bug.

Fixes: 5d171050e28f ("sparc64: vcc: Enable VCC port probe and removal")
Signed-off-by: Liang He <windhl@126.com>
---
 drivers/tty/vcc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/tty/vcc.c b/drivers/tty/vcc.c
index 34ba6e54789a..e3ba63d0a91f 100644
--- a/drivers/tty/vcc.c
+++ b/drivers/tty/vcc.c
@@ -610,6 +610,9 @@ static int vcc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
 
 	hp = mdesc_grab();
 
+	if (!hp)
+		return -ENODEV;
+
 	node = vio_vdev_node(hp, vdev);
 	if (node == MDESC_NODE_NULL) {
 		rv = -ENXIO;
-- 
2.25.1

Re: [PATCH] tty: vcc: add check for mdesc_grab()

[Greg KH]

On Wed, Mar 15, 2023 at 02:11:21PM +0800, Liang He wrote:
> In vcc_probe(), we should check the return value of
> mdesc_grab() as it may return NULL. While the
> vio_vdev_node() has the NULL-check, but if there
> is still a call to mdesc_release() which may cause
> a NPD bug.

Have you actually triggered this issue?  If so, how?

> Fixes: 5d171050e28f ("sparc64: vcc: Enable VCC port probe and removal")
> Signed-off-by: Liang He <windhl@126.com>
> ---
>  drivers/tty/vcc.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/tty/vcc.c b/drivers/tty/vcc.c
> index 34ba6e54789a..e3ba63d0a91f 100644
> --- a/drivers/tty/vcc.c
> +++ b/drivers/tty/vcc.c
> @@ -610,6 +610,9 @@ static int vcc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
>  
>  	hp = mdesc_grab();
>  
> +	if (!hp)
> +		return -ENODEV;

This change is obviously not correct and has not been tested, sorry.
Please do not make changes like this without properly validating them.

greg k-h

Re:Re: [PATCH] tty: vcc: add check for mdesc_grab()

[Liang He]

At 2023-03-15 14:32:54, "Greg KH" <gregkh@linuxfoundation.org> wrote:
>On Wed, Mar 15, 2023 at 02:11:21PM +0800, Liang He wrote:
>> In vcc_probe(), we should check the return value of
>> mdesc_grab() as it may return NULL. While the
>> vio_vdev_node() has the NULL-check, but if there
>> is still a call to mdesc_release() which may cause
>> a NPD bug.
>
>Have you actually triggered this issue?  If so, how?
>

Hi, Greg,
Thanks very much for your reply.

In fact, I have not actually triggered this issue, 
but I indeed meet lots of checks in other *_init functions, e.g., mdesc_adi_init()/ldc_init().

However, if we can make sure the return value can never be NULL 
when our kernel execute into these *_probe functions, my patchs are indeed useless.

Thanks and sorry for any potential trouble.

Liang 

>> Fixes: 5d171050e28f ("sparc64: vcc: Enable VCC port probe and removal")
>> Signed-off-by: Liang He <windhl@126.com>
>> ---
>>  drivers/tty/vcc.c | 3 +++
>>  1 file changed, 3 insertions(+)
>> 
>> diff --git a/drivers/tty/vcc.c b/drivers/tty/vcc.c
>> index 34ba6e54789a..e3ba63d0a91f 100644
>> --- a/drivers/tty/vcc.c
>> +++ b/drivers/tty/vcc.c
>> @@ -610,6 +610,9 @@ static int vcc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
>>  
>>  	hp = mdesc_grab();
>>  
>> +	if (!hp)
>> +		return -ENODEV;
>
>This change is obviously not correct and has not been tested, sorry.
>Please do not make changes like this without properly validating them.
>
>greg k-h