[PATCH][next] ALSA: vmaster: Use flex_array_size() helper in memcpy()
Make use of the flex_array_size() helper to calculate the size of a flexible array member within an enclosing structure.
This helper offers defense-in-depth against potential integer overflows and makes it explicitly clear that we are dealing with a flexible array member.
Signed-off-by: Gustavo A. R. Silva gustavoars@kernel.org --- sound/core/vmaster.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/core/vmaster.c b/sound/core/vmaster.c index ab36f9898711..21ce4082cb5f 100644 --- a/sound/core/vmaster.c +++ b/sound/core/vmaster.c @@ -262,7 +262,8 @@ int _snd_ctl_add_follower(struct snd_kcontrol *master, return -ENOMEM; srec->kctl = follower; srec->follower = *follower; - memcpy(srec->follower.vd, follower->vd, follower->count * sizeof(*follower->vd)); + memcpy(srec->follower.vd, follower->vd, flex_array_size(srec, follower.vd, + srec->follower.count)); srec->master = master_link; srec->flags = flags;
On Thu, 30 Jul 2020 00:18:29 +0200, Gustavo A. R. Silva wrote:
Make use of the flex_array_size() helper to calculate the size of a flexible array member within an enclosing structure.
This helper offers defense-in-depth against potential integer overflows and makes it explicitly clear that we are dealing with a flexible array member.
Signed-off-by: Gustavo A. R. Silva gustavoars@kernel.org
sound/core/vmaster.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/core/vmaster.c b/sound/core/vmaster.c index ab36f9898711..21ce4082cb5f 100644 --- a/sound/core/vmaster.c +++ b/sound/core/vmaster.c @@ -262,7 +262,8 @@ int _snd_ctl_add_follower(struct snd_kcontrol *master, return -ENOMEM; srec->kctl = follower; srec->follower = *follower;
- memcpy(srec->follower.vd, follower->vd, follower->count * sizeof(*follower->vd));
- memcpy(srec->follower.vd, follower->vd, flex_array_size(srec, follower.vd,
srec->follower.count));
Changing from follower->count to srec->follower.count isn't obvious, so it should have been mentioned in the log; other than that, the code change looks good.
But since the API isn't in Linus tree yet, I'll postpone the merge until the API reaches to upstream. Maybe I can merge this during RC1 merge window, as those are trivial.
BTW, looking at those patterns, I wonder whether it'd make sense to make the whole memset() call as a macro like
safe_copy_array(srec->follower.vd, follower->vd, follower->count);
?
thanks,
Takashi
On 7/30/20 03:41, Takashi Iwai wrote:
On Thu, 30 Jul 2020 00:18:29 +0200, Gustavo A. R. Silva wrote:
Make use of the flex_array_size() helper to calculate the size of a flexible array member within an enclosing structure.
This helper offers defense-in-depth against potential integer overflows and makes it explicitly clear that we are dealing with a flexible array member.
Signed-off-by: Gustavo A. R. Silva gustavoars@kernel.org
sound/core/vmaster.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/core/vmaster.c b/sound/core/vmaster.c index ab36f9898711..21ce4082cb5f 100644 --- a/sound/core/vmaster.c +++ b/sound/core/vmaster.c @@ -262,7 +262,8 @@ int _snd_ctl_add_follower(struct snd_kcontrol *master, return -ENOMEM; srec->kctl = follower; srec->follower = *follower;
- memcpy(srec->follower.vd, follower->vd, follower->count * sizeof(*follower->vd));
- memcpy(srec->follower.vd, follower->vd, flex_array_size(srec, follower.vd,
srec->follower.count));Changing from follower->count to srec->follower.count isn't obvious, so it should have been mentioned in the log; other than that, the code change looks good.
Yeah; you're right. I'll include that in the changelog text.
But since the API isn't in Linus tree yet, I'll postpone the merge until the API reaches to upstream. Maybe I can merge this during RC1 merge window, as those are trivial.
The API is already in mainline, see: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/incl...
BTW, looking at those patterns, I wonder whether it'd make sense to make the whole memset() call as a macro like
safe_copy_array(srec->follower.vd, follower->vd, follower->count);
?
Yep; I was actually thinking about that, yesterday. I might implement it for the next development cycle. :)
Thanks for the feedback. -- Gustavo
On Thu, 30 Jul 2020 15:12:33 +0200, Gustavo A. R. Silva wrote:
On 7/30/20 03:41, Takashi Iwai wrote:
On Thu, 30 Jul 2020 00:18:29 +0200, Gustavo A. R. Silva wrote:
Make use of the flex_array_size() helper to calculate the size of a flexible array member within an enclosing structure.
This helper offers defense-in-depth against potential integer overflows and makes it explicitly clear that we are dealing with a flexible array member.
Signed-off-by: Gustavo A. R. Silva gustavoars@kernel.org
sound/core/vmaster.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/core/vmaster.c b/sound/core/vmaster.c index ab36f9898711..21ce4082cb5f 100644 --- a/sound/core/vmaster.c +++ b/sound/core/vmaster.c @@ -262,7 +262,8 @@ int _snd_ctl_add_follower(struct snd_kcontrol *master, return -ENOMEM; srec->kctl = follower; srec->follower = *follower;
- memcpy(srec->follower.vd, follower->vd, follower->count * sizeof(*follower->vd));
- memcpy(srec->follower.vd, follower->vd, flex_array_size(srec, follower.vd,
srec->follower.count));Changing from follower->count to srec->follower.count isn't obvious, so it should have been mentioned in the log; other than that, the code change looks good.
Yeah; you're right. I'll include that in the changelog text.
OK, then I'll wait for the v2 for merging.
But since the API isn't in Linus tree yet, I'll postpone the merge until the API reaches to upstream. Maybe I can merge this during RC1 merge window, as those are trivial.
The API is already in mainline, see: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/incl...
Ah, thanks, I didn't expect that the API change comes after rc1 into mainline. Then I can merge later the commits based on the late rc.
Takashi
participants (3)
-
Gustavo A. R. Silva -
Gustavo A. R. Silva -
Takashi Iwai