linux-kernel - Re: [PATCH v1] media: rc-core: Modify the timeout waiting time for the infrared remote control.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Zv2xGbdhm8kXgDFe@gofer.mess.org>
Date: Wed, 2 Oct 2024 21:46:17 +0100
From: Sean Young <sean@...s.org>
To: Shen Lichuan <shenlichuan@...o.com>
Cc: mchehab@...nel.org, huanglipeng@...o.com, linux-media@...r.kernel.org,
	linux-kernel@...r.kernel.org, opensource.kernel@...o.com
Subject: Re: [PATCH v1] media: rc-core: Modify the timeout waiting time for
 the infrared remote control.

On Fri, Sep 27, 2024 at 06:58:08PM +0800, Shen Lichuan wrote:
> When transmitting codes from certain infrared remote controls, the kernel
> occasionally fails to receive them due to a timeout during transmission.
> 
> This issue arises specifically in instances where the duration of the 
> signal exceeds the predefined limit (`IR_MAX_DURATION`) in the code
> handling logic located within `lirc_dev.c`:
> 
> if (txbuf[i] > IR_MAX_DURATION - duration || !txbuf[i]) {
> 	pr_err("lirc_transmit duration out range[%d] txbuf:%d duration:%d\n",
> 		i, txbuf[i], duration);
> 	ret = -EINVAL;
> 	goto out_kfree;
> }
> 
> The error manifests as an `EINVAL` (error number 22) being returned when
> attempting to send infrared signals whose individual elements exceed the
> maximum allowed duration (`xbuf[i] > IR_MAX_DURATION - duration`).
> 
> As evidenced by logs, attempts to send commands with extended durations,
> such as those associated with the "Power" button on a Skyworth TV remote,
> fail with this error.
> 
> To rectify this and ensure compatibility with a broader range of infrared
> remote controls, particularly those with lengthy code sequences, this patch
> proposes to increase the value of `IR_MAX_DURATION`. 

IR_MAX_DURATION is already half second; can you elaborate on the signal
that the "Power" button on a Skyworth TV remote looks like? I doubt that
a signal button press would produce more than 500ms of IR; that would be
a lot IR for a single button, and would also have terrible latency for the
user.

My guess is that the signal is repeating, and you're trying to send
the signals with the repeats in one go. It would be nice to hear what
protocol this is and what it looks like encoded.

If the signal contains large gaps/spaces, then the signal can be split
up into multiple sends. For example, if you have this signal

+100 -150000 +150

You can also send this like so (pseudo code):

int fd = open("/dev/lirc0", O_RW);
write(fd, [100], 1);
usleep(150000);
write(fd, [100], 1);
close(fd);

This overcomes the limitation of the IR_MAX_DURATION, and also makes it
possible to send on much larger variety of infrared hardware, lots of them
do not support sending large gaps or long signals.

> This adjustment will allow for successful transmission of these extended
> codes, thereby enhancing overall device compatibility and ensuring proper
> functionality of remotes with long duration signals.
> 
> Example log entries highlighting the issue:
> 	D ConsumerIrHal: IRTX: Send to driver <268>
> 	E ConsumerIrHal: irtx write fail, errno=22 <269>
> 	D ConsumerIrHal: Done, Turn OFF IRTX <270>

What software is this, anything you can share about it?

> Modifying the maximum timeout time in this area can solve this issue.

We hold various locks during the transmit, and keeping it to a minimum
is much nicer. The gpio-ir-tx driver disables interrupts for this duration,
and many other drivers hold the rcdev mutex.

Thanks,

Sean

> 
> Signed-off-by: Huang Lipeng <huanglipeng@...o.com>
> Signed-off-by: Shen Lichuan <shenlichuan@...o.com>
> ---
>  include/media/rc-core.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/media/rc-core.h b/include/media/rc-core.h
> index d095908073ef..2f575c18b6b6 100644
> --- a/include/media/rc-core.h
> +++ b/include/media/rc-core.h
> @@ -303,7 +303,7 @@ struct ir_raw_event {
>  
>  #define US_TO_NS(usec)		((usec) * 1000)
>  #define MS_TO_US(msec)		((msec) * 1000)
> -#define IR_MAX_DURATION		MS_TO_US(500)
> +#define IR_MAX_DURATION		MS_TO_US(1000)
>  #define IR_DEFAULT_TIMEOUT	MS_TO_US(125)
>  #define IR_MAX_TIMEOUT		LIRC_VALUE_MASK
>  
> -- 
> 2.17.1