* [alsa-devel] [PATCH] ALSA: ctl: allow TLV read operation for callback type of element in locked case @ 2019-12-23 9:33 Takashi Sakamoto 2019-12-23 9:42 ` Takashi Sakamoto ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: Takashi Sakamoto @ 2019-12-23 9:33 UTC (permalink / raw) To: tiwai; +Cc: alsa-devel A design of ALSA control core allows applications to execute three operations for TLV feature; read, write and command. Furthermore, it allows driver developers to process the operations by two ways; allocated array or callback function. In the former, read operation is just allowed, thus developers uses the latter when device driver supports variety of models or the target model is expected to dynamically change information stored in TLV container. The core also allows applications to lock any element so that the other applications can't perform write operation to the element for element value and TLV information. When the element is locked, write and command operation for TLV information are prohibited as well as element value. Any read operation should be allowed in the case. At present, when an element has callback function for TLV information, TLV read operation returns EPERM if the element is locked. On the other hand, the read operation is success when an element has allocated array for TLV information. In both cases, read operation is success for element value expectedly. This commit fixes the bug. This change can be backported to v4.14 kernel or later. --- sound/core/control.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sound/core/control.c b/sound/core/control.c index 3fa1171dc1c2..49f3428dfacb 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -1398,8 +1398,9 @@ static int call_tlv_handler(struct snd_ctl_file *file, int op_flag, if (kctl->tlv.c == NULL) return -ENXIO; - /* When locked, this is unavailable. */ - if (vd->owner != NULL && vd->owner != file) + // Write and command operations are not allowed for locked element. + if (op_flag != SNDRV_CTL_TLV_OP_READ && + vd->owner != NULL && vd->owner != file) return -EPERM; return kctl->tlv.c(kctl, op_flag, size, buf); -- 2.20.1 _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [alsa-devel] [PATCH] ALSA: ctl: allow TLV read operation for callback type of element in locked case 2019-12-23 9:33 [alsa-devel] [PATCH] ALSA: ctl: allow TLV read operation for callback type of element in locked case Takashi Sakamoto @ 2019-12-23 9:42 ` Takashi Sakamoto 2019-12-23 15:03 ` Takashi Iwai 2019-12-24 12:56 ` Jaroslav Kysela 2 siblings, 0 replies; 6+ messages in thread From: Takashi Sakamoto @ 2019-12-23 9:42 UTC (permalink / raw) To: tiwai; +Cc: alsa-devel On Mon, Dec 23, 2019 at 06:33:47PM +0900, Takashi Sakamoto wrote: > At present, when an element has callback function for TLV information, > TLV read operation returns EPERM if the element is locked. On the > other hand, the read operation is success when an element has allocated > array for TLV information. In both cases, read operation is success for > element value expectedly. You can regenerate the issue by executing below Python 3 scripts: (Installation of alsa-gobject[1], PyGObject[2] and amixer(1) in alsa-utils is required.) After element is locked, amixer reports EPERM for any read operation of TLV information. ``` #########after locked######### numid=28,iface=PCM,name='Playback Channel Map' ; type=INTEGER,access=r---lR--,values=6,min=0,max=36,step=0 : values=0,0,0,0,0,0 amixer: Control hw:1 element TLV read error: Operation not permitted ``` ======== 8< -------- #!/usr/bin/env python3 from sys import argv,exit import subprocess import gi gi.require_version('ALSACtl', '0.0') from gi.repository import ALSACtl def lock_elems(card, targets, locked): for target in targets: card.lock_elem(target, locked) def run_amixer(label, targets): print('{:#^30}'.format(label)) args = ['amixer', '-c', str(card_id), 'cget', '(placeholder)'] for target in targets: args[4] = "iface=PCM,name='{}'".format(target.get_name()) subprocess.run(args) if len(argv) < 2: print('One argument is required for the numerical ID of soundcard.') exit(1) card_id = int(argv[1]) card = ALSACtl.Card.new() card.open(card_id) targets = list(filter(lambda e: e.get_name().find("Channel Map") >= 0, card.get_elem_id_list())) run_amixer('before locked', targets) lock_elems(card, targets, True) run_amixer('after locked', targets) lock_elems(card, targets, False) run_amixer('after released', targets) # The lock is surely released automatically when control character device is # released. Thus it's safe to terminate the above codes in its middle. ======== 8< -------- [1] https://github.com/alsa-project/alsa-gobject/ [2] https://pygobject.readthedocs.io/en/latest/ _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [alsa-devel] [PATCH] ALSA: ctl: allow TLV read operation for callback type of element in locked case 2019-12-23 9:33 [alsa-devel] [PATCH] ALSA: ctl: allow TLV read operation for callback type of element in locked case Takashi Sakamoto 2019-12-23 9:42 ` Takashi Sakamoto @ 2019-12-23 15:03 ` Takashi Iwai 2019-12-24 10:02 ` Takashi Sakamoto 2019-12-24 12:56 ` Jaroslav Kysela 2 siblings, 1 reply; 6+ messages in thread From: Takashi Iwai @ 2019-12-23 15:03 UTC (permalink / raw) To: Takashi Sakamoto; +Cc: alsa-devel On Mon, 23 Dec 2019 10:33:47 +0100, Takashi Sakamoto wrote: > > A design of ALSA control core allows applications to execute three > operations for TLV feature; read, write and command. Furthermore, it > allows driver developers to process the operations by two ways; allocated > array or callback function. In the former, read operation is just allowed, > thus developers uses the latter when device driver supports variety of > models or the target model is expected to dynamically change information > stored in TLV container. > > The core also allows applications to lock any element so that the other > applications can't perform write operation to the element for element > value and TLV information. When the element is locked, write and command > operation for TLV information are prohibited as well as element value. > Any read operation should be allowed in the case. > > At present, when an element has callback function for TLV information, > TLV read operation returns EPERM if the element is locked. On the > other hand, the read operation is success when an element has allocated > array for TLV information. In both cases, read operation is success for > element value expectedly. > > This commit fixes the bug. This change can be backported to v4.14 > kernel or later. The patch looks good but your sign-off is missing... thanks, Takashi _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [alsa-devel] [PATCH] ALSA: ctl: allow TLV read operation for callback type of element in locked case 2019-12-23 15:03 ` Takashi Iwai @ 2019-12-24 10:02 ` Takashi Sakamoto 2019-12-24 14:31 ` Takashi Iwai 0 siblings, 1 reply; 6+ messages in thread From: Takashi Sakamoto @ 2019-12-24 10:02 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel On Mon, Dec 23, 2019 at 04:03:53PM +0100, Takashi Iwai wrote: > On Mon, 23 Dec 2019 10:33:47 +0100, > Takashi Sakamoto wrote: > > > > A design of ALSA control core allows applications to execute three > > operations for TLV feature; read, write and command. Furthermore, it > > allows driver developers to process the operations by two ways; allocated > > array or callback function. In the former, read operation is just allowed, > > thus developers uses the latter when device driver supports variety of > > models or the target model is expected to dynamically change information > > stored in TLV container. > > > > The core also allows applications to lock any element so that the other > > applications can't perform write operation to the element for element > > value and TLV information. When the element is locked, write and command > > operation for TLV information are prohibited as well as element value. > > Any read operation should be allowed in the case. > > > > At present, when an element has callback function for TLV information, > > TLV read operation returns EPERM if the element is locked. On the > > other hand, the read operation is success when an element has allocated > > array for TLV information. In both cases, read operation is success for > > element value expectedly. > > > > This commit fixes the bug. This change can be backported to v4.14 > > kernel or later. > > The patch looks good but your sign-off is missing... Oops, I was in the festive mood... Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp> Besides, let us backport this patch to older kernels? As long as I investigate, this bug exists since v2.6.19 kernel, in which TLV feature was firstly introduced and extended[1]. When I worked for refactoring to control core in v4,14 kernel, the bug remains kept as is[2]. It's possible to apply this patch to the longterm v4.14.160 kernel. But when fixing this bug for the older kernels. we need to prepare an alternative patch. In my opinion, the disadvantage of the bug is not so critical, thus it's reasonable to abandon the older kernels. [1] https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/commit/?h=8aa9b586e4209 [2] https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/commit/?h=450296f305f13 Regards Takashi Sakamoto _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [alsa-devel] [PATCH] ALSA: ctl: allow TLV read operation for callback type of element in locked case 2019-12-24 10:02 ` Takashi Sakamoto @ 2019-12-24 14:31 ` Takashi Iwai 0 siblings, 0 replies; 6+ messages in thread From: Takashi Iwai @ 2019-12-24 14:31 UTC (permalink / raw) To: Takashi Sakamoto; +Cc: alsa-devel On Tue, 24 Dec 2019 11:02:10 +0100, Takashi Sakamoto wrote: > > On Mon, Dec 23, 2019 at 04:03:53PM +0100, Takashi Iwai wrote: > > On Mon, 23 Dec 2019 10:33:47 +0100, > > Takashi Sakamoto wrote: > > > > > > A design of ALSA control core allows applications to execute three > > > operations for TLV feature; read, write and command. Furthermore, it > > > allows driver developers to process the operations by two ways; allocated > > > array or callback function. In the former, read operation is just allowed, > > > thus developers uses the latter when device driver supports variety of > > > models or the target model is expected to dynamically change information > > > stored in TLV container. > > > > > > The core also allows applications to lock any element so that the other > > > applications can't perform write operation to the element for element > > > value and TLV information. When the element is locked, write and command > > > operation for TLV information are prohibited as well as element value. > > > Any read operation should be allowed in the case. > > > > > > At present, when an element has callback function for TLV information, > > > TLV read operation returns EPERM if the element is locked. On the > > > other hand, the read operation is success when an element has allocated > > > array for TLV information. In both cases, read operation is success for > > > element value expectedly. > > > > > > This commit fixes the bug. This change can be backported to v4.14 > > > kernel or later. > > > > The patch looks good but your sign-off is missing... > > Oops, I was in the festive mood... > > Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp> > > Besides, let us backport this patch to older kernels? As long as I > investigate, this bug exists since v2.6.19 kernel, in which TLV feature > was firstly introduced and extended[1]. When I worked for refactoring to > control core in v4,14 kernel, the bug remains kept as is[2]. > > It's possible to apply this patch to the longterm v4.14.160 kernel. But > when fixing this bug for the older kernels. we need to prepare an > alternative patch. In my opinion, the disadvantage of the bug is not so > critical, thus it's reasonable to abandon the older kernels. I don't think we need a stable backport. It's no regression, as you pointed, and nothing leading to a serious problem like a crash. So I merged now to for-next branch for 5.6 branch without Cc-to-stable. Also corrected the comment in C style as Jaroslav suggested, too. thanks, Takashi > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/commit/?h=8aa9b586e4209 > [2] https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/commit/?h=450296f305f13 > > > Regards > > Takashi Sakamoto > _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [alsa-devel] [PATCH] ALSA: ctl: allow TLV read operation for callback type of element in locked case 2019-12-23 9:33 [alsa-devel] [PATCH] ALSA: ctl: allow TLV read operation for callback type of element in locked case Takashi Sakamoto 2019-12-23 9:42 ` Takashi Sakamoto 2019-12-23 15:03 ` Takashi Iwai @ 2019-12-24 12:56 ` Jaroslav Kysela 2 siblings, 0 replies; 6+ messages in thread From: Jaroslav Kysela @ 2019-12-24 12:56 UTC (permalink / raw) To: alsa-devel; +Cc: Takashi Iwai Dne 23. 12. 19 v 10:33 Takashi Sakamoto napsal(a): > A design of ALSA control core allows applications to execute three > operations for TLV feature; read, write and command. Furthermore, it > allows driver developers to process the operations by two ways; allocated > array or callback function. In the former, read operation is just allowed, > thus developers uses the latter when device driver supports variety of > models or the target model is expected to dynamically change information > stored in TLV container. > > The core also allows applications to lock any element so that the other > applications can't perform write operation to the element for element > value and TLV information. When the element is locked, write and command > operation for TLV information are prohibited as well as element value. > Any read operation should be allowed in the case. > > At present, when an element has callback function for TLV information, > TLV read operation returns EPERM if the element is locked. On the > other hand, the read operation is success when an element has allocated > array for TLV information. In both cases, read operation is success for > element value expectedly. > > This commit fixes the bug. This change can be backported to v4.14 > kernel or later. > --- > sound/core/control.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/sound/core/control.c b/sound/core/control.c > index 3fa1171dc1c2..49f3428dfacb 100644 > --- a/sound/core/control.c > +++ b/sound/core/control.c > @@ -1398,8 +1398,9 @@ static int call_tlv_handler(struct snd_ctl_file *file, int op_flag, > if (kctl->tlv.c == NULL) > return -ENXIO; > > - /* When locked, this is unavailable. */ > - if (vd->owner != NULL && vd->owner != file) > + // Write and command operations are not allowed for locked element. > + if (op_flag != SNDRV_CTL_TLV_OP_READ && > + vd->owner != NULL && vd->owner != file) > return -EPERM; > > return kctl->tlv.c(kctl, op_flag, size, buf); > Thanks. Good catch. I would change only the comment back to /* */ to be consistent with the other sound/core code. Reviewed-by: Jaroslav Kysela <perex@perex.cz> -- Jaroslav Kysela <perex@perex.cz> Linux Sound Maintainer; ALSA Project; Red Hat, Inc. _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-12-24 14:32 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-23 9:33 [alsa-devel] [PATCH] ALSA: ctl: allow TLV read operation for callback type of element in locked case Takashi Sakamoto 2019-12-23 9:42 ` Takashi Sakamoto 2019-12-23 15:03 ` Takashi Iwai 2019-12-24 10:02 ` Takashi Sakamoto 2019-12-24 14:31 ` Takashi Iwai 2019-12-24 12:56 ` Jaroslav Kysela
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).