Dear Linux folks,
Running the ALSA kselftests with Linux 6.10-rc1, `mixer-test` shows ten failures:
# Totals: pass:24 fail:0 xfail:0 xpass:0 skip:11 error:0
These are:
not ok 5 write_invalid.0.40 not ok 11 write_valid.0.39 not ok 18 write_valid.0.38 not ok 25 write_valid.0.37 not ok 201 write_invalid.0.12 not ok 208 write_invalid.0.11 not ok 264 write_invalid.0.3 not ok 271 write_invalid.0.2 not ok 278 write_invalid.0.1 not ok 285 write_invalid.0.0
`./pcm-test` finishes with 5 out of 5 passes.
The output `sudo alsa-info` is uploaded to the database [1], and the full output of `mixer-test` is attached.
Kind regards,
Paul
[1]: https://alsa-project.org/db/?f=cf8e1e0f37775448b5e3361e5c3cd4d4e6037d4f
On Fri, 31 May 2024 07:50:33 +0200, Paul Menzel wrote:
Dear Linux folks,
Running the ALSA kselftests with Linux 6.10-rc1, `mixer-test` shows ten failures:
# Totals: pass:24 fail:0 xfail:0 xpass:0 skip:11 error:0These are:
not ok 5 write_invalid.0.40 not ok 11 write_valid.0.39 not ok 18 write_valid.0.38 not ok 25 write_valid.0.37 not ok 201 write_invalid.0.12 not ok 208 write_invalid.0.11 not ok 264 write_invalid.0.3 not ok 271 write_invalid.0.2 not ok 278 write_invalid.0.1 not ok 285 write_invalid.0.0
Through a quick look, those are no real "failures". It'd be more preferable if the driver returns an error for invalid values, but currently it's up to drivers how to deal with them, and some accept as is but with correction of the values internally. They are shown as "skips" in the summary above you showed, after all.
A more strict check can be enabled by a kconfig option CONFIG_SND_CTL_INPUT_VALIDATION=y generically.
thanks,
Takashi
Dear Takashi,
Thank you very much for your reply.
Am 31.05.24 um 17:17 schrieb Takashi Iwai:
On Fri, 31 May 2024 07:50:33 +0200, Paul Menzel wrote:
Running the ALSA kselftests with Linux 6.10-rc1, `mixer-test` shows ten failures:
# Totals: pass:24 fail:0 xfail:0 xpass:0 skip:11 error:0These are:
not ok 5 write_invalid.0.40 not ok 11 write_valid.0.39 not ok 18 write_valid.0.38 not ok 25 write_valid.0.37 not ok 201 write_invalid.0.12 not ok 208 write_invalid.0.11 not ok 264 write_invalid.0.3 not ok 271 write_invalid.0.2 not ok 278 write_invalid.0.1 not ok 285 write_invalid.0.0Through a quick look, those are no real "failures". It'd be more preferable if the driver returns an error for invalid values, but currently it's up to drivers how to deal with them, and some accept as is but with correction of the values internally. They are shown as "skips" in the summary above you showed, after all.
Sorry, somehow I copied the wrong line. The attachment actually contains:
# Totals: pass:217 fail:10 xfail:0 xpass:0 skip:60 error:0
So it seems to be shown as failure.
A more strict check can be enabled by a kconfig option CONFIG_SND_CTL_INPUT_VALIDATION=y generically.
Thank you. I am going to test that out.
Kind regards,
Paul
On Fri, May 31, 2024 at 05:17:43PM +0200, Takashi Iwai wrote:
On Fri, 31 May 2024 07:50:33 +0200,
not ok 5 write_invalid.0.40 not ok 201 write_invalid.0.12 not ok 208 write_invalid.0.11 not ok 264 write_invalid.0.3 not ok 271 write_invalid.0.2 not ok 278 write_invalid.0.1 not ok 285 write_invalid.0.0
Through a quick look, those are no real "failures". It'd be more preferable if the driver returns an error for invalid values, but currently it's up to drivers how to deal with them, and some accept as is but with correction of the values internally. They are shown as "skips" in the summary above you showed, after all.
I would say these are all bugs, they show the driver not correcting the value and allowing users to read back out of range values that were written. Even if the driver is accepting out of range values I'd expect it to transform them somehow when storing, the program will accept a mismatched read when testing this case but it will complain if the read value is not valid according to the control's info.
On Fri, 31 May 2024 18:03:52 +0200, Mark Brown wrote:
On Fri, May 31, 2024 at 05:17:43PM +0200, Takashi Iwai wrote:
On Fri, 31 May 2024 07:50:33 +0200,
not ok 5 write_invalid.0.40 not ok 201 write_invalid.0.12 not ok 208 write_invalid.0.11 not ok 264 write_invalid.0.3 not ok 271 write_invalid.0.2 not ok 278 write_invalid.0.1 not ok 285 write_invalid.0.0Through a quick look, those are no real "failures". It'd be more preferable if the driver returns an error for invalid values, but currently it's up to drivers how to deal with them, and some accept as is but with correction of the values internally. They are shown as "skips" in the summary above you showed, after all.
I would say these are all bugs, they show the driver not correcting the value and allowing users to read back out of range values that were written. Even if the driver is accepting out of range values I'd expect it to transform them somehow when storing, the program will accept a mismatched read when testing this case but it will complain if the read value is not valid according to the control's info.
Ideally, yeah. But it's a whack-a-mole game, and my gut feeling is that it'd be better to enable the input validation globally, something like below.
Takashi
--- a/sound/core/Kconfig +++ b/sound/core/Kconfig @@ -219,7 +219,8 @@ config SND_PCM_XRUN_DEBUG the process or driver which causes the scheduling gaps.
config SND_CTL_INPUT_VALIDATION - bool "Validate input data to control API" + bool "Validate input data to control API" if EXPERT + default y help Say Y to enable the additional validation for the input data to each control element, including the value range checks.
On Fri, May 31, 2024 at 08:06:14PM +0200, Takashi Iwai wrote:
Mark Brown wrote:
On Fri, May 31, 2024 at 05:17:43PM +0200, Takashi Iwai wrote:
On Fri, 31 May 2024 07:50:33 +0200,
I would say these are all bugs, they show the driver not correcting the value and allowing users to read back out of range values that were written. Even if the driver is accepting out of range values I'd expect it to transform them somehow when storing, the program will accept a mismatched read when testing this case but it will complain if the read value is not valid according to the control's info.
Ideally, yeah. But it's a whack-a-mole game, and my gut feeling is that it'd be better to enable the input validation globally, something like below.
Yeah, I mean I tend to think the whole accepting invalid values thing is questionable to start off with so I do think that's a good idea. That said we probably should still be fixing the drivers as well.
On Mon, 03 Jun 2024 13:38:18 +0200, Mark Brown wrote:
On Fri, May 31, 2024 at 08:06:14PM +0200, Takashi Iwai wrote:
Mark Brown wrote:
On Fri, May 31, 2024 at 05:17:43PM +0200, Takashi Iwai wrote:
On Fri, 31 May 2024 07:50:33 +0200,
I would say these are all bugs, they show the driver not correcting the value and allowing users to read back out of range values that were written. Even if the driver is accepting out of range values I'd expect it to transform them somehow when storing, the program will accept a mismatched read when testing this case but it will complain if the read value is not valid according to the control's info.
Ideally, yeah. But it's a whack-a-mole game, and my gut feeling is that it'd be better to enable the input validation globally, something like below.
Yeah, I mean I tend to think the whole accepting invalid values thing is questionable to start off with so I do think that's a good idea. That said we probably should still be fixing the drivers as well.
OK, I'm going to submit a patch set for addressing those.
Takashi
On 14. 06. 24 13:33, Takashi Iwai wrote:
On Mon, 03 Jun 2024 13:38:18 +0200, Mark Brown wrote:
On Fri, May 31, 2024 at 08:06:14PM +0200, Takashi Iwai wrote:
Mark Brown wrote:
On Fri, May 31, 2024 at 05:17:43PM +0200, Takashi Iwai wrote:
On Fri, 31 May 2024 07:50:33 +0200,
I would say these are all bugs, they show the driver not correcting the value and allowing users to read back out of range values that were written. Even if the driver is accepting out of range values I'd expect it to transform them somehow when storing, the program will accept a mismatched read when testing this case but it will complain if the read value is not valid according to the control's info.
Ideally, yeah. But it's a whack-a-mole game, and my gut feeling is that it'd be better to enable the input validation globally, something like below.
Yeah, I mean I tend to think the whole accepting invalid values thing is questionable to start off with so I do think that's a good idea. That said we probably should still be fixing the drivers as well.
OK, I'm going to submit a patch set for addressing those.
I think that this check should be optional (as discussed some years ago), because the driver code can implement the validation / bitmask handling in a more efficient way that we can do in the ALSA core code. Those double checks are not so nice. But they may be enabled by default as suggested to log problems for users building custom kernels, IMHO.
Jaroslav
On Fri, 14 Jun 2024 13:42:05 +0200, Jaroslav Kysela wrote:
On 14. 06. 24 13:33, Takashi Iwai wrote:
On Mon, 03 Jun 2024 13:38:18 +0200, Mark Brown wrote:
On Fri, May 31, 2024 at 08:06:14PM +0200, Takashi Iwai wrote:
Mark Brown wrote:
On Fri, May 31, 2024 at 05:17:43PM +0200, Takashi Iwai wrote:
On Fri, 31 May 2024 07:50:33 +0200,
I would say these are all bugs, they show the driver not correcting the value and allowing users to read back out of range values that were written. Even if the driver is accepting out of range values I'd expect it to transform them somehow when storing, the program will accept a mismatched read when testing this case but it will complain if the read value is not valid according to the control's info.
Ideally, yeah. But it's a whack-a-mole game, and my gut feeling is that it'd be better to enable the input validation globally, something like below.
Yeah, I mean I tend to think the whole accepting invalid values thing is questionable to start off with so I do think that's a good idea. That said we probably should still be fixing the drivers as well.
OK, I'm going to submit a patch set for addressing those.
I think that this check should be optional (as discussed some years ago), because the driver code can implement the validation / bitmask handling in a more efficient way that we can do in the ALSA core code. Those double checks are not so nice. But they may be enabled by default as suggested to log problems for users building custom kernels, IMHO.
Yes, what I've worked on are the coverage in HD-audio and vmaster code as well as the enablement of validation for user control elements. It's not about unconditional enablement of input validation.
thanks,
Takashi
linux-kselftest-mirror@lists.linaro.org
-
Jaroslav Kysela -
Mark Brown -
Paul Menzel -
Takashi Iwai