All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] scan-build fixes
@ 2020-12-26 21:35 Alex Henrie
  2020-12-26 21:35 ` [PATCH 1/9] conf: fix use after free in _snd_config_load_with_include Alex Henrie
                   ` (8 more replies)
  0 siblings, 9 replies; 24+ messages in thread
From: Alex Henrie @ 2020-12-26 21:35 UTC (permalink / raw)
  To: alsa-devel, perex, tiwai; +Cc: Alex Henrie

These defects were identified by scan-build
<https://clang-analyzer.llvm.org/scan-build.html>.

I hope that drawing attention to the problems helps even if you decide
to fix some of them in different ways than I am proposing.

Happy holidays!

-Alex

Alex Henrie (9):
  conf: fix use after free in _snd_config_load_with_include
  ucm: fix bad frees in get_list0 and get_list20
  rawmidi: fix memory leak in snd_rawmidi_virtual_open
  confmisc: fix memory leak in snd_func_concat
  timer: fix sizeof operator mismatch in snd_timer_query_hw_open
  pcm: remove dead assignments from
    snd_pcm_rate_(commit_area|grab_next_period)
  pcm_multi: remove dead assignment from _snd_pcm_multi_open
  pcm: fix undefined bit shift in bad_pcm_state
  conf: remove dead code from get_hexachar

 include/pcm.h              |  4 +++-
 src/conf.c                 | 13 ++++---------
 src/confmisc.c             |  2 +-
 src/pcm/pcm.c              |  2 ++
 src/pcm/pcm_local.h        |  2 +-
 src/pcm/pcm_multi.c        |  1 -
 src/pcm/pcm_rate.c         |  2 --
 src/rawmidi/rawmidi_virt.c |  3 ++-
 src/timer/timer_query_hw.c |  2 +-
 src/ucm/main.c             |  4 ++--
 10 files changed, 16 insertions(+), 19 deletions(-)

-- 
2.29.2.368.ge46b91665e.dirty


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 1/9] conf: fix use after free in _snd_config_load_with_include
  2020-12-26 21:35 [PATCH 0/9] scan-build fixes Alex Henrie
@ 2020-12-26 21:35 ` Alex Henrie
  2020-12-27  8:38   ` Takashi Iwai
  2020-12-26 21:35 ` [PATCH 2/9] ucm: fix bad frees in get_list0 and get_list20 Alex Henrie
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Alex Henrie @ 2020-12-26 21:35 UTC (permalink / raw)
  To: alsa-devel, perex, tiwai; +Cc: Alex Henrie

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 src/conf.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/conf.c b/src/conf.c
index 7df2b4e7..44d1bfde 100644
--- a/src/conf.c
+++ b/src/conf.c
@@ -1970,7 +1970,9 @@ int _snd_config_load_with_include(snd_config_t *config, snd_input_t *in,
 		SNDERR("%s:%d:%d:%s", fd->name ? fd->name : "_toplevel_", fd->line, fd->column, str);
 		goto _end;
 	}
-	if (get_char(&input) != LOCAL_UNEXPECTED_EOF) {
+	err = get_char(&input);
+	fd = input.current;
+	if (err != LOCAL_UNEXPECTED_EOF) {
 		SNDERR("%s:%d:%d:Unexpected }", fd->name ? fd->name : "", fd->line, fd->column);
 		err = -EINVAL;
 		goto _end;
-- 
2.29.2.368.ge46b91665e.dirty


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 2/9] ucm: fix bad frees in get_list0 and get_list20
  2020-12-26 21:35 [PATCH 0/9] scan-build fixes Alex Henrie
  2020-12-26 21:35 ` [PATCH 1/9] conf: fix use after free in _snd_config_load_with_include Alex Henrie
@ 2020-12-26 21:35 ` Alex Henrie
  2020-12-27  8:38   ` Takashi Iwai
  2020-12-26 21:35 ` [PATCH 3/9] rawmidi: fix memory leak in snd_rawmidi_virtual_open Alex Henrie
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Alex Henrie @ 2020-12-26 21:35 UTC (permalink / raw)
  To: alsa-devel, perex, tiwai; +Cc: Alex Henrie

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 src/ucm/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/ucm/main.c b/src/ucm/main.c
index 3871d5aa..754b967e 100644
--- a/src/ucm/main.c
+++ b/src/ucm/main.c
@@ -666,7 +666,7 @@ static int get_list0(struct list_head *list,
 	}
 	return cnt;
       __fail:
-        snd_use_case_free_list((const char **)res, cnt);
+        snd_use_case_free_list(*result, cnt);
         return -ENOMEM;
 }
 
@@ -724,7 +724,7 @@ static int get_list20(struct list_head *list,
 	}
 	return cnt;
       __fail:
-        snd_use_case_free_list((const char **)res, cnt);
+        snd_use_case_free_list(*result, cnt);
         return -ENOMEM;
 }
 
-- 
2.29.2.368.ge46b91665e.dirty


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 3/9] rawmidi: fix memory leak in snd_rawmidi_virtual_open
  2020-12-26 21:35 [PATCH 0/9] scan-build fixes Alex Henrie
  2020-12-26 21:35 ` [PATCH 1/9] conf: fix use after free in _snd_config_load_with_include Alex Henrie
  2020-12-26 21:35 ` [PATCH 2/9] ucm: fix bad frees in get_list0 and get_list20 Alex Henrie
@ 2020-12-26 21:35 ` Alex Henrie
  2020-12-27  8:38   ` Takashi Iwai
  2020-12-26 21:35 ` [PATCH 4/9] confmisc: fix memory leak in snd_func_concat Alex Henrie
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Alex Henrie @ 2020-12-26 21:35 UTC (permalink / raw)
  To: alsa-devel, perex, tiwai; +Cc: Alex Henrie

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 src/rawmidi/rawmidi_virt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/rawmidi/rawmidi_virt.c b/src/rawmidi/rawmidi_virt.c
index 2c4c27f5..884b8ff8 100644
--- a/src/rawmidi/rawmidi_virt.c
+++ b/src/rawmidi/rawmidi_virt.c
@@ -315,7 +315,7 @@ int snd_rawmidi_virtual_open(snd_rawmidi_t **inputp, snd_rawmidi_t **outputp,
 			     int merge, int mode)
 {
 	int err;
-	snd_rawmidi_t *rmidi;
+	snd_rawmidi_t *rmidi = NULL;
 	snd_rawmidi_virtual_t *virt = NULL;
 	struct pollfd pfd;
 
@@ -392,6 +392,7 @@ int snd_rawmidi_virtual_open(snd_rawmidi_t **inputp, snd_rawmidi_t **outputp,
 		free(*inputp);
 	if (outputp)
 		free(*outputp);
+	free(rmidi);
 	return err;
 }
 
-- 
2.29.2.368.ge46b91665e.dirty


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 4/9] confmisc: fix memory leak in snd_func_concat
  2020-12-26 21:35 [PATCH 0/9] scan-build fixes Alex Henrie
                   ` (2 preceding siblings ...)
  2020-12-26 21:35 ` [PATCH 3/9] rawmidi: fix memory leak in snd_rawmidi_virtual_open Alex Henrie
@ 2020-12-26 21:35 ` Alex Henrie
  2020-12-27  8:26   ` Takashi Iwai
  2020-12-26 21:35 ` [PATCH 5/9] timer: fix sizeof operator mismatch in snd_timer_query_hw_open Alex Henrie
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Alex Henrie @ 2020-12-26 21:35 UTC (permalink / raw)
  To: alsa-devel, perex, tiwai; +Cc: Alex Henrie

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 src/confmisc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/confmisc.c b/src/confmisc.c
index eb8218c1..aece39c3 100644
--- a/src/confmisc.c
+++ b/src/confmisc.c
@@ -440,8 +440,8 @@ int snd_func_concat(snd_config_t **dst, snd_config_t *root, snd_config_t *src,
 	err = snd_config_get_id(src, &id);
 	if (err >= 0)
 		err = snd_config_imake_string(dst, id, res);
-	free(res);
       __error:
+	free(res);
 	return err;
 }
 #ifndef DOC_HIDDEN
-- 
2.29.2.368.ge46b91665e.dirty


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 5/9] timer: fix sizeof operator mismatch in snd_timer_query_hw_open
  2020-12-26 21:35 [PATCH 0/9] scan-build fixes Alex Henrie
                   ` (3 preceding siblings ...)
  2020-12-26 21:35 ` [PATCH 4/9] confmisc: fix memory leak in snd_func_concat Alex Henrie
@ 2020-12-26 21:35 ` Alex Henrie
  2020-12-27  8:39   ` Takashi Iwai
  2020-12-26 21:35 ` [PATCH 6/9] pcm: remove dead assignments from snd_pcm_rate_(commit_area|grab_next_period) Alex Henrie
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Alex Henrie @ 2020-12-26 21:35 UTC (permalink / raw)
  To: alsa-devel, perex, tiwai; +Cc: Alex Henrie

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 src/timer/timer_query_hw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/timer/timer_query_hw.c b/src/timer/timer_query_hw.c
index dad228c8..d8bac6e7 100644
--- a/src/timer/timer_query_hw.c
+++ b/src/timer/timer_query_hw.c
@@ -104,7 +104,7 @@ int snd_timer_query_hw_open(snd_timer_query_t **handle, const char *name, int mo
 		close(fd);
 		return -SND_ERROR_INCOMPATIBLE_VERSION;
 	}
-	tmr = (snd_timer_query_t *) calloc(1, sizeof(snd_timer_t));
+	tmr = (snd_timer_query_t *) calloc(1, sizeof(snd_timer_query_t));
 	if (tmr == NULL) {
 		close(fd);
 		return -ENOMEM;
-- 
2.29.2.368.ge46b91665e.dirty


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 6/9] pcm: remove dead assignments from snd_pcm_rate_(commit_area|grab_next_period)
  2020-12-26 21:35 [PATCH 0/9] scan-build fixes Alex Henrie
                   ` (4 preceding siblings ...)
  2020-12-26 21:35 ` [PATCH 5/9] timer: fix sizeof operator mismatch in snd_timer_query_hw_open Alex Henrie
@ 2020-12-26 21:35 ` Alex Henrie
  2020-12-27  8:39   ` Takashi Iwai
  2020-12-26 21:35 ` [PATCH 7/9] pcm_multi: remove dead assignment from _snd_pcm_multi_open Alex Henrie
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Alex Henrie @ 2020-12-26 21:35 UTC (permalink / raw)
  To: alsa-devel, perex, tiwai; +Cc: Alex Henrie

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 src/pcm/pcm_rate.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/pcm/pcm_rate.c b/src/pcm/pcm_rate.c
index 5bf7dbb9..dc38e95e 100644
--- a/src/pcm/pcm_rate.c
+++ b/src/pcm/pcm_rate.c
@@ -746,7 +746,6 @@ static int snd_pcm_rate_commit_area(snd_pcm_t *pcm, snd_pcm_rate_t *rate,
 		if (result < 0)
 			return result;
 	      __partial:
-		xfer = 0;
 		cont = slave_frames;
 		if (cont > slave_size)
 			cont = slave_size;
@@ -846,7 +845,6 @@ static int snd_pcm_rate_grab_next_period(snd_pcm_t *pcm, snd_pcm_uframes_t hw_of
 		if (result < 0)
 			return result;
 	      __partial:
-		xfer = 0;
 		cont = slave_frames;
 		if (cont > rate->gen.slave->period_size)
 			cont = rate->gen.slave->period_size;
-- 
2.29.2.368.ge46b91665e.dirty


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 7/9] pcm_multi: remove dead assignment from _snd_pcm_multi_open
  2020-12-26 21:35 [PATCH 0/9] scan-build fixes Alex Henrie
                   ` (5 preceding siblings ...)
  2020-12-26 21:35 ` [PATCH 6/9] pcm: remove dead assignments from snd_pcm_rate_(commit_area|grab_next_period) Alex Henrie
@ 2020-12-26 21:35 ` Alex Henrie
  2020-12-27  8:39   ` Takashi Iwai
  2020-12-26 21:35 ` [PATCH 8/9] pcm: fix undefined bit shift in bad_pcm_state Alex Henrie
  2020-12-26 21:35 ` [PATCH 9/9] conf: remove dead code from get_hexachar Alex Henrie
  8 siblings, 1 reply; 24+ messages in thread
From: Alex Henrie @ 2020-12-26 21:35 UTC (permalink / raw)
  To: alsa-devel, perex, tiwai; +Cc: Alex Henrie

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 src/pcm/pcm_multi.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/pcm/pcm_multi.c b/src/pcm/pcm_multi.c
index 53c414d5..5fa09b9b 100644
--- a/src/pcm/pcm_multi.c
+++ b/src/pcm/pcm_multi.c
@@ -1323,7 +1323,6 @@ int _snd_pcm_multi_open(snd_pcm_t **pcmp, const char *name,
 		err = -ENOMEM;
 		goto _free;
 	}
-	idx = 0;
 	for (idx = 0; idx < channels_count; ++idx)
 		channels_sidx[idx] = -1;
 	idx = 0;
-- 
2.29.2.368.ge46b91665e.dirty


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 8/9] pcm: fix undefined bit shift in bad_pcm_state
  2020-12-26 21:35 [PATCH 0/9] scan-build fixes Alex Henrie
                   ` (6 preceding siblings ...)
  2020-12-26 21:35 ` [PATCH 7/9] pcm_multi: remove dead assignment from _snd_pcm_multi_open Alex Henrie
@ 2020-12-26 21:35 ` Alex Henrie
  2020-12-27  8:34   ` Takashi Iwai
  2020-12-26 21:35 ` [PATCH 9/9] conf: remove dead code from get_hexachar Alex Henrie
  8 siblings, 1 reply; 24+ messages in thread
From: Alex Henrie @ 2020-12-26 21:35 UTC (permalink / raw)
  To: alsa-devel, perex, tiwai; +Cc: Alex Henrie

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 include/pcm.h       | 4 +++-
 src/pcm/pcm.c       | 2 ++
 src/pcm/pcm_local.h | 2 +-
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/pcm.h b/include/pcm.h
index e300b951..c6c5d8f8 100644
--- a/include/pcm.h
+++ b/include/pcm.h
@@ -307,7 +307,9 @@ typedef enum _snd_pcm_state {
 	SND_PCM_STATE_SUSPENDED,
 	/** Hardware is disconnected */
 	SND_PCM_STATE_DISCONNECTED,
-	SND_PCM_STATE_LAST = SND_PCM_STATE_DISCONNECTED,
+	/** State cannot be queried */
+	SND_PCM_STATE_UNKNOWN,
+	SND_PCM_STATE_LAST = SND_PCM_STATE_UNKNOWN,
 	/** Private - used internally in the library - do not use*/
 	SND_PCM_STATE_PRIVATE1 = 1024
 } snd_pcm_state_t;
diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c
index 24030b31..5fafc2a0 100644
--- a/src/pcm/pcm.c
+++ b/src/pcm/pcm.c
@@ -680,6 +680,8 @@ static int pcm_state_to_error(snd_pcm_state_t state)
 		return -ESTRPIPE;
 	case SND_PCM_STATE_DISCONNECTED:
 		return -ENODEV;
+	case SND_PCM_STATE_UNKNOWN:
+		return -ENOSYS;
 	default:
 		return 0;
 	}
diff --git a/src/pcm/pcm_local.h b/src/pcm/pcm_local.h
index fe77e50d..04f89623 100644
--- a/src/pcm/pcm_local.h
+++ b/src/pcm/pcm_local.h
@@ -444,7 +444,7 @@ static inline int __snd_pcm_start(snd_pcm_t *pcm)
 static inline snd_pcm_state_t __snd_pcm_state(snd_pcm_t *pcm)
 {
 	if (!pcm->fast_ops->state)
-		return -ENOSYS;
+		return SND_PCM_STATE_UNKNOWN;
 	return pcm->fast_ops->state(pcm->fast_op_arg);
 }
 
-- 
2.29.2.368.ge46b91665e.dirty


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 9/9] conf: remove dead code from get_hexachar
  2020-12-26 21:35 [PATCH 0/9] scan-build fixes Alex Henrie
                   ` (7 preceding siblings ...)
  2020-12-26 21:35 ` [PATCH 8/9] pcm: fix undefined bit shift in bad_pcm_state Alex Henrie
@ 2020-12-26 21:35 ` Alex Henrie
  2020-12-27  8:37   ` Takashi Iwai
  8 siblings, 1 reply; 24+ messages in thread
From: Alex Henrie @ 2020-12-26 21:35 UTC (permalink / raw)
  To: alsa-devel, perex, tiwai; +Cc: Alex Henrie

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 src/conf.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/src/conf.c b/src/conf.c
index 44d1bfde..970ad643 100644
--- a/src/conf.c
+++ b/src/conf.c
@@ -877,16 +877,9 @@ static int get_nonwhite(input_t *input)
 
 static inline int get_hexachar(input_t *input)
 {
-	int c, num = 0;
-
+	int c;
 	c = get_char(input);
-	if (c >= '0' && c <= '9') num |= (c - '0') << 4;
-	else if (c >= 'a' && c <= 'f') num |= (c - 'a') << 4;
-	else if (c >= 'A' && c <= 'F') num |= (c - 'A') << 4;
 	c = get_char(input);
-	if (c >= '0' && c <= '9') num |= (c - '0') << 0;
-	else if (c >= 'a' && c <= 'f') num |= (c - 'a') << 0;
-	else if (c >= 'A' && c <= 'F') num |= (c - 'A') << 0;
 	return c;
 }
 
-- 
2.29.2.368.ge46b91665e.dirty


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH 4/9] confmisc: fix memory leak in snd_func_concat
  2020-12-26 21:35 ` [PATCH 4/9] confmisc: fix memory leak in snd_func_concat Alex Henrie
@ 2020-12-27  8:26   ` Takashi Iwai
  2020-12-28  1:45     ` Alex Henrie
  0 siblings, 1 reply; 24+ messages in thread
From: Takashi Iwai @ 2020-12-27  8:26 UTC (permalink / raw)
  To: Alex Henrie; +Cc: alsa-devel

On Sat, 26 Dec 2020 22:35:42 +0100,
Alex Henrie wrote:
> 
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> ---
>  src/confmisc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/confmisc.c b/src/confmisc.c
> index eb8218c1..aece39c3 100644
> --- a/src/confmisc.c
> +++ b/src/confmisc.c
> @@ -440,8 +440,8 @@ int snd_func_concat(snd_config_t **dst, snd_config_t *root, snd_config_t *src,
>  	err = snd_config_get_id(src, &id);
>  	if (err >= 0)
>  		err = snd_config_imake_string(dst, id, res);
> -	free(res);
>        __error:
> +	free(res);
>  	return err;
>  }
>  #ifndef DOC_HIDDEN

I guess this would lead to the double-free at the error path after
realloc() that already freed res before the goto line.
Care to fix it and resubmit this one?


thanks,

Takashi

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 8/9] pcm: fix undefined bit shift in bad_pcm_state
  2020-12-26 21:35 ` [PATCH 8/9] pcm: fix undefined bit shift in bad_pcm_state Alex Henrie
@ 2020-12-27  8:34   ` Takashi Iwai
  2020-12-27 12:36     ` Jaroslav Kysela
  0 siblings, 1 reply; 24+ messages in thread
From: Takashi Iwai @ 2020-12-27  8:34 UTC (permalink / raw)
  To: Alex Henrie; +Cc: alsa-devel

On Sat, 26 Dec 2020 22:35:46 +0100,
Alex Henrie wrote:
> 
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> ---
>  include/pcm.h       | 4 +++-
>  src/pcm/pcm.c       | 2 ++
>  src/pcm/pcm_local.h | 2 +-
>  3 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/include/pcm.h b/include/pcm.h
> index e300b951..c6c5d8f8 100644
> --- a/include/pcm.h
> +++ b/include/pcm.h
> @@ -307,7 +307,9 @@ typedef enum _snd_pcm_state {
>  	SND_PCM_STATE_SUSPENDED,
>  	/** Hardware is disconnected */
>  	SND_PCM_STATE_DISCONNECTED,
> -	SND_PCM_STATE_LAST = SND_PCM_STATE_DISCONNECTED,
> +	/** State cannot be queried */
> +	SND_PCM_STATE_UNKNOWN,
> +	SND_PCM_STATE_LAST = SND_PCM_STATE_UNKNOWN,
>  	/** Private - used internally in the library - do not use*/
>  	SND_PCM_STATE_PRIVATE1 = 1024
>  } snd_pcm_state_t;

We can't add a random new state here.  If any, such a thing has to be
defined locally, but this would also break ABI.  So, I don't think
adding a new state is the right fix.

> diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c
> index 24030b31..5fafc2a0 100644
> --- a/src/pcm/pcm.c
> +++ b/src/pcm/pcm.c
> @@ -680,6 +680,8 @@ static int pcm_state_to_error(snd_pcm_state_t state)
>  		return -ESTRPIPE;
>  	case SND_PCM_STATE_DISCONNECTED:
>  		return -ENODEV;
> +	case SND_PCM_STATE_UNKNOWN:
> +		return -ENOSYS;
>  	default:
>  		return 0;
>  	}
> diff --git a/src/pcm/pcm_local.h b/src/pcm/pcm_local.h
> index fe77e50d..04f89623 100644
> --- a/src/pcm/pcm_local.h
> +++ b/src/pcm/pcm_local.h
> @@ -444,7 +444,7 @@ static inline int __snd_pcm_start(snd_pcm_t *pcm)
>  static inline snd_pcm_state_t __snd_pcm_state(snd_pcm_t *pcm)
>  {
>  	if (!pcm->fast_ops->state)
> -		return -ENOSYS;
> +		return SND_PCM_STATE_UNKNOWN;
>  	return pcm->fast_ops->state(pcm->fast_op_arg);

We need either to handle a special error value in all places calling
__snd_pcm_state() or to just return SND_PCM_STATE_XRUN or such instead
here, IMO.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 9/9] conf: remove dead code from get_hexachar
  2020-12-26 21:35 ` [PATCH 9/9] conf: remove dead code from get_hexachar Alex Henrie
@ 2020-12-27  8:37   ` Takashi Iwai
  2020-12-27 12:25     ` Jaroslav Kysela
  0 siblings, 1 reply; 24+ messages in thread
From: Takashi Iwai @ 2020-12-27  8:37 UTC (permalink / raw)
  To: Alex Henrie; +Cc: alsa-devel

On Sat, 26 Dec 2020 22:35:47 +0100,
Alex Henrie wrote:
> 
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> ---
>  src/conf.c | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/src/conf.c b/src/conf.c
> index 44d1bfde..970ad643 100644
> --- a/src/conf.c
> +++ b/src/conf.c
> @@ -877,16 +877,9 @@ static int get_nonwhite(input_t *input)
>  
>  static inline int get_hexachar(input_t *input)
>  {
> -	int c, num = 0;
> -
> +	int c;
>  	c = get_char(input);
> -	if (c >= '0' && c <= '9') num |= (c - '0') << 4;
> -	else if (c >= 'a' && c <= 'f') num |= (c - 'a') << 4;
> -	else if (c >= 'A' && c <= 'F') num |= (c - 'A') << 4;
>  	c = get_char(input);
> -	if (c >= '0' && c <= '9') num |= (c - '0') << 0;
> -	else if (c >= 'a' && c <= 'f') num |= (c - 'a') << 0;
> -	else if (c >= 'A' && c <= 'F') num |= (c - 'A') << 0;
>  	return c;

The current code is obviously wrong and the suggested fix goes even to
a wronger direction :)  The function should return num instead.

I wonder how this did't hit any problem, so far.  Maybe 0x prefix was
rarely used, fortunately.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/9] conf: fix use after free in _snd_config_load_with_include
  2020-12-26 21:35 ` [PATCH 1/9] conf: fix use after free in _snd_config_load_with_include Alex Henrie
@ 2020-12-27  8:38   ` Takashi Iwai
  0 siblings, 0 replies; 24+ messages in thread
From: Takashi Iwai @ 2020-12-27  8:38 UTC (permalink / raw)
  To: Alex Henrie; +Cc: alsa-devel

On Sat, 26 Dec 2020 22:35:39 +0100,
Alex Henrie wrote:
> 
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>

Applied, thanks.


Takashi

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/9] ucm: fix bad frees in get_list0 and get_list20
  2020-12-26 21:35 ` [PATCH 2/9] ucm: fix bad frees in get_list0 and get_list20 Alex Henrie
@ 2020-12-27  8:38   ` Takashi Iwai
  0 siblings, 0 replies; 24+ messages in thread
From: Takashi Iwai @ 2020-12-27  8:38 UTC (permalink / raw)
  To: Alex Henrie; +Cc: alsa-devel

On Sat, 26 Dec 2020 22:35:40 +0100,
Alex Henrie wrote:
> 
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>

Applied, thanks.


Takashi

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/9] rawmidi: fix memory leak in snd_rawmidi_virtual_open
  2020-12-26 21:35 ` [PATCH 3/9] rawmidi: fix memory leak in snd_rawmidi_virtual_open Alex Henrie
@ 2020-12-27  8:38   ` Takashi Iwai
  0 siblings, 0 replies; 24+ messages in thread
From: Takashi Iwai @ 2020-12-27  8:38 UTC (permalink / raw)
  To: Alex Henrie; +Cc: alsa-devel

On Sat, 26 Dec 2020 22:35:41 +0100,
Alex Henrie wrote:
> 
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>

Applied, thanks.


Takashi

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 5/9] timer: fix sizeof operator mismatch in snd_timer_query_hw_open
  2020-12-26 21:35 ` [PATCH 5/9] timer: fix sizeof operator mismatch in snd_timer_query_hw_open Alex Henrie
@ 2020-12-27  8:39   ` Takashi Iwai
  0 siblings, 0 replies; 24+ messages in thread
From: Takashi Iwai @ 2020-12-27  8:39 UTC (permalink / raw)
  To: Alex Henrie; +Cc: alsa-devel

On Sat, 26 Dec 2020 22:35:43 +0100,
Alex Henrie wrote:
> 
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>

Applied, thanks.


Takashi

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 6/9] pcm: remove dead assignments from snd_pcm_rate_(commit_area|grab_next_period)
  2020-12-26 21:35 ` [PATCH 6/9] pcm: remove dead assignments from snd_pcm_rate_(commit_area|grab_next_period) Alex Henrie
@ 2020-12-27  8:39   ` Takashi Iwai
  0 siblings, 0 replies; 24+ messages in thread
From: Takashi Iwai @ 2020-12-27  8:39 UTC (permalink / raw)
  To: Alex Henrie; +Cc: alsa-devel

On Sat, 26 Dec 2020 22:35:44 +0100,
Alex Henrie wrote:
> 
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>

Applied, thanks.


Takashi

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 7/9] pcm_multi: remove dead assignment from _snd_pcm_multi_open
  2020-12-26 21:35 ` [PATCH 7/9] pcm_multi: remove dead assignment from _snd_pcm_multi_open Alex Henrie
@ 2020-12-27  8:39   ` Takashi Iwai
  0 siblings, 0 replies; 24+ messages in thread
From: Takashi Iwai @ 2020-12-27  8:39 UTC (permalink / raw)
  To: Alex Henrie; +Cc: alsa-devel

On Sat, 26 Dec 2020 22:35:45 +0100,
Alex Henrie wrote:
> 
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>

Applied, thanks.


Takashi

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 9/9] conf: remove dead code from get_hexachar
  2020-12-27  8:37   ` Takashi Iwai
@ 2020-12-27 12:25     ` Jaroslav Kysela
  2020-12-28  1:42       ` Alex Henrie
  0 siblings, 1 reply; 24+ messages in thread
From: Jaroslav Kysela @ 2020-12-27 12:25 UTC (permalink / raw)
  To: Takashi Iwai, Alex Henrie; +Cc: alsa-devel

Dne 27. 12. 20 v 9:37 Takashi Iwai napsal(a):
> On Sat, 26 Dec 2020 22:35:47 +0100,
> Alex Henrie wrote:
>>
>> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
>> ---
>>  src/conf.c | 9 +--------
>>  1 file changed, 1 insertion(+), 8 deletions(-)
>>
>> diff --git a/src/conf.c b/src/conf.c
>> index 44d1bfde..970ad643 100644
>> --- a/src/conf.c
>> +++ b/src/conf.c
>> @@ -877,16 +877,9 @@ static int get_nonwhite(input_t *input)
>>  
>>  static inline int get_hexachar(input_t *input)
>>  {
>> -	int c, num = 0;
>> -
>> +	int c;
>>  	c = get_char(input);
>> -	if (c >= '0' && c <= '9') num |= (c - '0') << 4;
>> -	else if (c >= 'a' && c <= 'f') num |= (c - 'a') << 4;
>> -	else if (c >= 'A' && c <= 'F') num |= (c - 'A') << 4;
>>  	c = get_char(input);
>> -	if (c >= '0' && c <= '9') num |= (c - '0') << 0;
>> -	else if (c >= 'a' && c <= 'f') num |= (c - 'a') << 0;
>> -	else if (c >= 'A' && c <= 'F') num |= (c - 'A') << 0;
>>  	return c;
> 
> The current code is obviously wrong and the suggested fix goes even to
> a wronger direction :)  The function should return num instead.
> 
> I wonder how this did't hit any problem, so far.  Maybe 0x prefix was
> rarely used, fortunately.

It's a bit recent code. I fixed the return value now. It's for \xFF not for
0xFF prefix. Thank you for your investigation, Alex.

					Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 8/9] pcm: fix undefined bit shift in bad_pcm_state
  2020-12-27  8:34   ` Takashi Iwai
@ 2020-12-27 12:36     ` Jaroslav Kysela
  2020-12-28  1:45       ` Alex Henrie
  0 siblings, 1 reply; 24+ messages in thread
From: Jaroslav Kysela @ 2020-12-27 12:36 UTC (permalink / raw)
  To: Takashi Iwai, Alex Henrie; +Cc: alsa-devel

Dne 27. 12. 20 v 9:34 Takashi Iwai napsal(a):
> We need either to handle a special error value in all places calling
> __snd_pcm_state() or to just return SND_PCM_STATE_XRUN or such instead
> here, IMO.

I think that SND_PCM_STATE_OPEN is more appropriate here. If the state
callback is not defined, the state management is screwed anyway. The other
functions will return an error (because they depend on the state management),
so it's safe. I applied this change to repo.

					Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 9/9] conf: remove dead code from get_hexachar
  2020-12-27 12:25     ` Jaroslav Kysela
@ 2020-12-28  1:42       ` Alex Henrie
  0 siblings, 0 replies; 24+ messages in thread
From: Alex Henrie @ 2020-12-28  1:42 UTC (permalink / raw)
  To: Jaroslav Kysela; +Cc: Takashi Iwai, alsa-devel

On Sun, Dec 27, 2020 at 5:25 AM Jaroslav Kysela <perex@perex.cz> wrote:
>
> Dne 27. 12. 20 v 9:37 Takashi Iwai napsal(a):
> >
> > The current code is obviously wrong and the suggested fix goes even to
> > a wronger direction :)  The function should return num instead.
> >
> > I wonder how this did't hit any problem, so far.  Maybe 0x prefix was
> > rarely used, fortunately.
>
> It's a bit recent code. I fixed the return value now. It's for \xFF not for
> 0xFF prefix. Thank you for your investigation, Alex.

Thank you for fixing this properly!

-Alex

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 8/9] pcm: fix undefined bit shift in bad_pcm_state
  2020-12-27 12:36     ` Jaroslav Kysela
@ 2020-12-28  1:45       ` Alex Henrie
  0 siblings, 0 replies; 24+ messages in thread
From: Alex Henrie @ 2020-12-28  1:45 UTC (permalink / raw)
  To: Jaroslav Kysela; +Cc: Takashi Iwai, alsa-devel

On Sun, Dec 27, 2020 at 5:36 AM Jaroslav Kysela <perex@perex.cz> wrote:
>
> Dne 27. 12. 20 v 9:34 Takashi Iwai napsal(a):
> > We need either to handle a special error value in all places calling
> > __snd_pcm_state() or to just return SND_PCM_STATE_XRUN or such instead
> > here, IMO.
>
> I think that SND_PCM_STATE_OPEN is more appropriate here. If the state
> callback is not defined, the state management is screwed anyway. The other
> functions will return an error (because they depend on the state management),
> so it's safe. I applied this change to repo.

Thank you for fixing this properly!

-Alex

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 4/9] confmisc: fix memory leak in snd_func_concat
  2020-12-27  8:26   ` Takashi Iwai
@ 2020-12-28  1:45     ` Alex Henrie
  0 siblings, 0 replies; 24+ messages in thread
From: Alex Henrie @ 2020-12-28  1:45 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel

On Sun, Dec 27, 2020 at 1:26 AM Takashi Iwai <tiwai@suse.de> wrote:
>
> I guess this would lead to the double-free at the error path after
> realloc() that already freed res before the goto line.
> Care to fix it and resubmit this one?

Thank you for catching that! I just sent a v2 of this patch.

-Alex

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2020-12-28  1:47 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-26 21:35 [PATCH 0/9] scan-build fixes Alex Henrie
2020-12-26 21:35 ` [PATCH 1/9] conf: fix use after free in _snd_config_load_with_include Alex Henrie
2020-12-27  8:38   ` Takashi Iwai
2020-12-26 21:35 ` [PATCH 2/9] ucm: fix bad frees in get_list0 and get_list20 Alex Henrie
2020-12-27  8:38   ` Takashi Iwai
2020-12-26 21:35 ` [PATCH 3/9] rawmidi: fix memory leak in snd_rawmidi_virtual_open Alex Henrie
2020-12-27  8:38   ` Takashi Iwai
2020-12-26 21:35 ` [PATCH 4/9] confmisc: fix memory leak in snd_func_concat Alex Henrie
2020-12-27  8:26   ` Takashi Iwai
2020-12-28  1:45     ` Alex Henrie
2020-12-26 21:35 ` [PATCH 5/9] timer: fix sizeof operator mismatch in snd_timer_query_hw_open Alex Henrie
2020-12-27  8:39   ` Takashi Iwai
2020-12-26 21:35 ` [PATCH 6/9] pcm: remove dead assignments from snd_pcm_rate_(commit_area|grab_next_period) Alex Henrie
2020-12-27  8:39   ` Takashi Iwai
2020-12-26 21:35 ` [PATCH 7/9] pcm_multi: remove dead assignment from _snd_pcm_multi_open Alex Henrie
2020-12-27  8:39   ` Takashi Iwai
2020-12-26 21:35 ` [PATCH 8/9] pcm: fix undefined bit shift in bad_pcm_state Alex Henrie
2020-12-27  8:34   ` Takashi Iwai
2020-12-27 12:36     ` Jaroslav Kysela
2020-12-28  1:45       ` Alex Henrie
2020-12-26 21:35 ` [PATCH 9/9] conf: remove dead code from get_hexachar Alex Henrie
2020-12-27  8:37   ` Takashi Iwai
2020-12-27 12:25     ` Jaroslav Kysela
2020-12-28  1:42       ` Alex Henrie

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.