| Message ID | 20210426145541.8070-1-lyl2019@mail.ustc.edu.cn (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4fb44dd2c1dda18606348acdfdb97e8759dde9df |
| Headers | show |
| Series | sound/isa/ib: Fix two use after free in snd_sb_qsound_build | expand |
On Mon, 26 Apr 2021 16:55:41 +0200, Lv Yunlong wrote: > > In snd_sb_qsound_build, snd_ctl_add(..,p->qsound_switch...) and > snd_ctl_add(..,p->qsound_space..) are called. But the second > arguments of snd_ctl_add() could be freed via snd_ctl_add_replace() > ->snd_ctl_free_one(). After the error code is returned, > snd_sb_qsound_destroy(p) is called in __error branch. > > But in snd_sb_qsound_destroy(), the freed p->qsound_switch and > p->qsound_space are still used by snd_ctl_remove(). > > My patch set p->qsound_switch and p->qsound_space to NULL if > snd_ctl_add() failed to avoid the uaf bugs. But these codes need > to further be improved with the code style. > > Signed-off-by: Lv Yunlong <lyl2019@mail.ustc.edu.cn> Thanks, applied now. Takashi
diff --git a/sound/isa/sb/sb16_csp.c b/sound/isa/sb/sb16_csp.c index 8635a2b6b36b..4789345a8fdd 100644 --- a/sound/isa/sb/sb16_csp.c +++ b/sound/isa/sb/sb16_csp.c @@ -1045,10 +1045,14 @@ static int snd_sb_qsound_build(struct snd_sb_csp * p) spin_lock_init(&p->q_lock); - if ((err = snd_ctl_add(card, p->qsound_switch = snd_ctl_new1(&snd_sb_qsound_switch, p))) < 0) + if ((err = snd_ctl_add(card, p->qsound_switch = snd_ctl_new1(&snd_sb_qsound_switch, p))) < 0) { + p->qsound_switch = NULL; goto __error; - if ((err = snd_ctl_add(card, p->qsound_space = snd_ctl_new1(&snd_sb_qsound_space, p))) < 0) + } + if ((err = snd_ctl_add(card, p->qsound_space = snd_ctl_new1(&snd_sb_qsound_space, p))) < 0) { + p->qsound_space = NULL; goto __error; + } return 0;
In snd_sb_qsound_build, snd_ctl_add(..,p->qsound_switch...) and snd_ctl_add(..,p->qsound_space..) are called. But the second arguments of snd_ctl_add() could be freed via snd_ctl_add_replace() ->snd_ctl_free_one(). After the error code is returned, snd_sb_qsound_destroy(p) is called in __error branch. But in snd_sb_qsound_destroy(), the freed p->qsound_switch and p->qsound_space are still used by snd_ctl_remove(). My patch set p->qsound_switch and p->qsound_space to NULL if snd_ctl_add() failed to avoid the uaf bugs. But these codes need to further be improved with the code style. Signed-off-by: Lv Yunlong <lyl2019@mail.ustc.edu.cn> --- sound/isa/sb/sb16_csp.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)