linux-kernel - Re: [PATCH 1/1] media: nec-decoder: remove trailer

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20180421102732.5sbi6nzfu33b435m@gofer.mess.org>
Date:   Sat, 21 Apr 2018 11:27:32 +0100
From:   Sean Young <sean@...s.org>
To:     Vladislav Zhurba <vzhurba@...dia.com>
Cc:     linux-kernel@...r.kernel.org, mchehab@...nel.org,
        linux-media@...r.kernel.org, Daniel Fu <danifu@...dia.com>
Subject: Re: [PATCH 1/1] media: nec-decoder: remove trailer_space state

On Fri, Apr 20, 2018 at 11:51:39AM -0700, Vladislav Zhurba wrote:
> From: Daniel Fu <danifu@...dia.com>
> 
> Remove STATE_TRAILER_SPACE from state machine.
> Causing 2 issue:
> - can not decode the keycode, if it didn't following with
>   another keycode/repeat code
> - will generate one more code in current logic.
>   i.e. key_right + repeat code + key_left + repeat code.
>   expect: key_right, key_left.
>   Result: key_right, key_right, key_right.
>   Reason: when receive repeat code of key_right, state machine will
>   stay in STATE_TRAILER_SPACE state, then wait for a new interrupt,
>   if an interrupt came after keyup_timer, then will generate another
>   fake key.

This behaviour is symptomatic of rc driver which does not generate
ir timeouts. If an rc driver does not do this, then it won't be just the
nec protocol which is not parsed correctly. For example, rc-6 in mode 6a
can have 16, 24 or 32 bits and we rely on the ir timeout to demarcate the
end of the IR message -- else we are stuck with the behaviour as you
describe above.

> According to the NEC protocol, it don't need a trailer space. Remove it.

This isn't the right solution, so NAK I'm afraid. 

Please let us know what rc driver you are using, I'm happy to help fix it.


Sean

> 
> Signed-off-by: Daniel Fu <danifu@...dia.com>
> Signed-off-by: Vladislav Zhurba <vzhurba@...dia.com>
> ---
>  drivers/media/rc/ir-nec-decoder.c | 10 ----------
>  1 file changed, 10 deletions(-)
> 
> diff --git a/drivers/media/rc/ir-nec-decoder.c b/drivers/media/rc/ir-nec-decoder.c
> index 21647b809e6f..760b66affd1a 100644
> --- a/drivers/media/rc/ir-nec-decoder.c
> +++ b/drivers/media/rc/ir-nec-decoder.c
> @@ -128,16 +128,6 @@ static int ir_nec_decode(struct rc_dev *dev, struct ir_raw_event ev)
>  		if (!eq_margin(ev.duration, NEC_TRAILER_PULSE, NEC_UNIT / 2))
>  			break;
>  
> -		data->state = STATE_TRAILER_SPACE;
> -		return 0;
> -
> -	case STATE_TRAILER_SPACE:
> -		if (ev.pulse)
> -			break;
> -
> -		if (!geq_margin(ev.duration, NEC_TRAILER_SPACE, NEC_UNIT / 2))
> -			break;
> -
>  		if (data->count == NEC_NBITS) {
>  			address     = bitrev8((data->bits >> 24) & 0xff);
>  			not_address = bitrev8((data->bits >> 16) & 0xff);
> -- 
> 2.16.2