[PATCH 0/2] staging: most: sound: implement improvements reported in code review

[PATCH 0/2] staging: most: sound: implement improvements reported in code review

[Christian Gromm]

This patch set fixes two problems found during code audit.

Christian Gromm (2):
  staging: most: sound: add sanity check for function argument
  staging: most: sound: use non-safe list iteration

 drivers/staging/most/sound/sound.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/2] staging: most: sound: add sanity check for function argument

[Christian Gromm]

This patch zero checks the function parameter 'bytes' before doing the
subtraction to prevent memory corruption.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/staging/most/sound/sound.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/most/sound/sound.c b/drivers/staging/most/sound/sound.c
index 3a1a590..953a4fe 100644
--- a/drivers/staging/most/sound/sound.c
+++ b/drivers/staging/most/sound/sound.c
@@ -86,6 +86,8 @@ static void swap_copy24(u8 *dest, const u8 *source, unsigned int bytes)
 {
 	unsigned int i = 0;
 
+	if (!bytes)
+		return;
 	while (i < bytes - 2) {
 		dest[i] = source[i + 2];
 		dest[i + 1] = source[i + 1];
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/2] staging: most: sound: use non-safe list iteration

[Christian Gromm]

This patch replaces the safe list iteration function with the
non-safe one, as no list element is being deleted.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/staging/most/sound/sound.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/most/sound/sound.c b/drivers/staging/most/sound/sound.c
index 953a4fe..9e23b11 100644
--- a/drivers/staging/most/sound/sound.c
+++ b/drivers/staging/most/sound/sound.c
@@ -160,9 +160,9 @@ static struct channel *get_channel(struct most_interface *iface,
 				   int channel_id)
 {
 	struct sound_adapter *adpt = iface->priv;
-	struct channel *channel, *tmp;
+	struct channel *channel;
 
-	list_for_each_entry_safe(channel, tmp, &adpt->dev_list, list) {
+	list_for_each_entry(channel, &adpt->dev_list, list) {
 		if ((channel->iface == iface) && (channel->id == channel_id))
 			return channel;
 	}
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] staging: most: sound: add sanity check for function argument

[Dan Carpenter]

On Tue, Feb 02, 2021 at 12:38:09PM +0100, Christian Gromm wrote:
> This patch zero checks the function parameter 'bytes' before doing the
> subtraction to prevent memory corruption.
> 
> Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
>  drivers/staging/most/sound/sound.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/staging/most/sound/sound.c b/drivers/staging/most/sound/sound.c
> index 3a1a590..953a4fe 100644
> --- a/drivers/staging/most/sound/sound.c
> +++ b/drivers/staging/most/sound/sound.c
> @@ -86,6 +86,8 @@ static void swap_copy24(u8 *dest, const u8 *source, unsigned int bytes)
>  {
>  	unsigned int i = 0;
>  
> +	if (!bytes)
> +		return;
>  	while (i < bytes - 2) {

If "bytes == 1" then this will cause problems still.  "bytes - 2"
becomes UINT_MAX.  I mean probably that's not possible but we may as
well make the sanity check if (bytes < 2) just for readability.

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel