[PATCH V3] ppdev: Add an error check in register_device

[PATCH V3] ppdev: Add an error check in register_device

[Huai-Yuan Liu]

In register_device, the return value of ida_simple_get is unchecked, 
in witch ida_simple_get will use an invalid index value.

To address this issue, index should be checked after ida_simple_get. When
the index value is abnormal, a warning message should be printed, the port
should be dropped, and the value should be recorded.

Fixes: 9a69645dde11 ("ppdev: fix registering same device name")
Signed-off-by: Huai-Yuan Liu <qq810974084@gmail.com>
---
V2:
* In patch V2, we found that parport_find_number implicitly calls
parport_get_port(). So when dealing with abnormal index values, we should
call parport_put_port() to throw away the reference to the port.
V3:
* In patch V3, we made some additional adjustments to the jump labels,
making the code more concise and readable.
  Thanks to Christophe JAILLET for helpful suggestion.
---
 drivers/char/ppdev.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/char/ppdev.c b/drivers/char/ppdev.c
index 4c188e9e477c..276db589b901 100644
--- a/drivers/char/ppdev.c
+++ b/drivers/char/ppdev.c
@@ -296,28 +296,36 @@ static int register_device(int minor, struct pp_struct *pp)
 	if (!port) {
 		pr_warn("%s: no associated port!\n", name);
 		rc = -ENXIO;
-		goto err;
+		goto err_free_name;
 	}
 
 	index = ida_simple_get(&ida_index, 0, 0, GFP_KERNEL);
+	if (index < 0) {
+		pr_warn("%s: failed to get index!\n", name);
+		rc = index;
+		goto err_put_port;
+	}
+
 	memset(&ppdev_cb, 0, sizeof(ppdev_cb));
 	ppdev_cb.irq_func = pp_irq;
 	ppdev_cb.flags = (pp->flags & PP_EXCL) ? PARPORT_FLAG_EXCL : 0;
 	ppdev_cb.private = pp;
 	pdev = parport_register_dev_model(port, name, &ppdev_cb, index);
-	parport_put_port(port);
 
 	if (!pdev) {
 		pr_warn("%s: failed to register device!\n", name);
 		rc = -ENXIO;
 		ida_simple_remove(&ida_index, index);
-		goto err;
+		goto err_put_port;
 	}
 
 	pp->pdev = pdev;
 	pp->index = index;
 	dev_dbg(&pdev->dev, "registered pardevice\n");
-err:
+
+err_put_port:
+	parport_put_port(port);
+err_free_name:
 	kfree(name);
 	return rc;
 }
-- 
2.34.1

Re: [PATCH V3] ppdev: Add an error check in register_device

[Greg KH]

On Sun, Apr 07, 2024 at 09:20:54PM +0800, Huai-Yuan Liu wrote:
> In register_device, the return value of ida_simple_get is unchecked, 
> in witch ida_simple_get will use an invalid index value.
> 
> To address this issue, index should be checked after ida_simple_get. When
> the index value is abnormal, a warning message should be printed, the port
> should be dropped, and the value should be recorded.
> 
> Fixes: 9a69645dde11 ("ppdev: fix registering same device name")
> Signed-off-by: Huai-Yuan Liu <qq810974084@gmail.com>
> ---
> V2:
> * In patch V2, we found that parport_find_number implicitly calls
> parport_get_port(). So when dealing with abnormal index values, we should
> call parport_put_port() to throw away the reference to the port.
> V3:
> * In patch V3, we made some additional adjustments to the jump labels,
> making the code more concise and readable.
>   Thanks to Christophe JAILLET for helpful suggestion.
> ---
>  drivers/char/ppdev.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/char/ppdev.c b/drivers/char/ppdev.c
> index 4c188e9e477c..276db589b901 100644
> --- a/drivers/char/ppdev.c
> +++ b/drivers/char/ppdev.c
> @@ -296,28 +296,36 @@ static int register_device(int minor, struct pp_struct *pp)
>  	if (!port) {
>  		pr_warn("%s: no associated port!\n", name);
>  		rc = -ENXIO;
> -		goto err;
> +		goto err_free_name;
>  	}
>  
>  	index = ida_simple_get(&ida_index, 0, 0, GFP_KERNEL);
> +	if (index < 0) {
> +		pr_warn("%s: failed to get index!\n", name);
> +		rc = index;
> +		goto err_put_port;
> +	}
> +
>  	memset(&ppdev_cb, 0, sizeof(ppdev_cb));
>  	ppdev_cb.irq_func = pp_irq;
>  	ppdev_cb.flags = (pp->flags & PP_EXCL) ? PARPORT_FLAG_EXCL : 0;
>  	ppdev_cb.private = pp;
>  	pdev = parport_register_dev_model(port, name, &ppdev_cb, index);
> -	parport_put_port(port);
>  
>  	if (!pdev) {
>  		pr_warn("%s: failed to register device!\n", name);
>  		rc = -ENXIO;
>  		ida_simple_remove(&ida_index, index);
> -		goto err;
> +		goto err_put_port;
>  	}
>  
>  	pp->pdev = pdev;
>  	pp->index = index;
>  	dev_dbg(&pdev->dev, "registered pardevice\n");
> -err:
> +
> +err_put_port:
> +	parport_put_port(port);
> +err_free_name:
>  	kfree(name);
>  	return rc;
>  }
> -- 
> 2.34.1
> 

Does not apply to the tree at all :(

Re: [PATCH V3] ppdev: Add an error check in register_device

[Huai-Yuan Liu]

Hello,

I offer my sincere apologies if my submission has brought you any
dissatisfaction. Could you please kindly inform me about the reasons
why this patch could not be accepted? Your reply would be greatly
appreciated.

Best regards,

Liu.

Re: [PATCH V3] ppdev: Add an error check in register_device

[Huai-Yuan Liu]

Hi,

I sincerely apologize for my negligence in not noticing the branch
changes,, resulting in this patch not being applicable to the tree. I
have regenerated a new patch and sent it. I apologize once again for
any trouble caused by my oversight. And thanks to Greg KH for pointing
this out.

Best regards,

Liu.