netdev - Re: rose timer t error displayed in /proc/net/rose

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 PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Mon, 1 Aug 2022 21:33:08 +0200
From:   Bernard f6bvp <f6bvp@...e.fr>
To:     Francois Romieu <romieu@...zoreil.com>,
        Thomas Osterried <thomas@...erried.de>
Cc:     Eric Dumazet <edumazet@...gle.com>, linux-hams@...r.kernel.org,
        Thomas Osterried DL9SAU <thomas@...erg.in-berlin.de>,
        netdev@...r.kernel.org
Subject: Re: rose timer t error displayed in /proc/net/rose

François, your new patch is working fine in both state 1 and 2 of rose.

Congrats and thanks !

By the way, it is a very old display issue... Already present in 5.4.79 
(32 bytes OS) for example and probably since much earlier.

Le 01/08/2022 à 17:44, Francois Romieu a écrit :
> Thomas Osterried <thomas@...erried.de> :
> [...]
>> 1. why do you check for pending timer anymore?
> s/do/don't/ ?
>
> I don't check for pending timer because:
> - the check is racy
> - jiffies_delta_to_clock_t maxes negative delta to zero
>
>> 2. I'm not really sure what value jiffies_delta_to_clock_t() returns. jiffies / HZ?
>>     jiffies_delta_to_clock_t() returns clock_t.
> I completely messed this part :o/
>
> clock_t relates to USER_HZ like jiffies does to HZ.
>
> [don't break the ax25]
>
> Sure, we are in violent agreement here.
>
> [...]
>> 1. I expect rose->timer to be restarted soon. If it does not happen, is there a bug?
> The relevant rose state in Bernard's sample was ROSE_STATE_2.
>
> net/rose/rose_timer.c::rose_timer_expiry would had called
> rose_disconnect(sk, ETIMEDOUT, -1, -1) so there should not
> be any timer restart (afaiu, etc.).
>
> [...]
>>     => Negative may be handled due to Francois' patch now correctly.
>> delta as signed long may be negative. max(0L, -nnnn) sould result to 0L.
>>     This would result to 0. Perhaps proven by Francois, because he used this function and achieved a correct display of that idle value. Francois, am I correct, is "0" really displayed?
> I must confess that I was not terribly professional this morning past 2AM.
>
> The attached snippet illustrates the behavior wrt negative values
> (make; insmod foo.ko ; sleep 1; rmmod foo.ko; dmesg | tail -n 2).
> It also illustrates that I got the unit wrong.
>
> This should be better:
>
> diff --git a/net/ax25/ax25_timer.c b/net/ax25/ax25_timer.c
> index 85865ebfdfa2..3c94e5a2d098 100644
> --- a/net/ax25/ax25_timer.c
> +++ b/net/ax25/ax25_timer.c
> @@ -108,10 +108,9 @@ int ax25_t1timer_running(ax25_cb *ax25)
>   
>   unsigned long ax25_display_timer(struct timer_list *timer)
>   {
> -	if (!timer_pending(timer))
> -		return 0;
> +	long delta = timer->expires - jiffies;
>   
> -	return timer->expires - jiffies;
> +	return max(0L, delta);
>   }
>   
>   EXPORT_SYMBOL(ax25_display_timer);
>