netdev - RE: [RFC net-next] xfrm: refactory to avoid state tasklet scheduling errors

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] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 14 Jul 2015 07:53:11 +0000
From:	"Du, Fan" <fan.du@...el.com>
To:	Giuseppe Cantavenera <giuseppe.cantavenera@...om.it>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:	Steffen Klassert <steffen.klassert@...unet.com>,
	"David S. Miller" <davem@...emloft.net>,
	Alexander Sverdlin <alexander.sverdlin@...ia.com>,
	Matija Glavinic Pecotic <matija.glavinic-pecotic.ext@...ia.com>,
	"Giuseppe Cantavenera" <giuseppe.cantavenera.ext@...ia.com>,
	Nicholas Faustini <nicholas.faustini.ext@...ia.com>,
	"Du, Fan" <fan.du@...el.com>
Subject: RE: [RFC net-next] xfrm: refactory to avoid state tasklet
 scheduling errors



>-----Original Message-----
>From: Giuseppe Cantavenera [mailto:giuseppe.cantavenera@...om.it]
>Sent: Tuesday, July 7, 2015 3:43 PM
>To: netdev@...r.kernel.org
>Cc: Giuseppe Cantavenera; Steffen Klassert; David S. Miller; Du, Fan; Alexander
>Sverdlin; Matija Glavinic Pecotic; Giuseppe Cantavenera; Nicholas Faustini
>Subject: [RFC net-next] xfrm: refactory to avoid state tasklet scheduling errors
>
>The SA state is managed by a tasklet scheduled relying on the wall clock.
>Previous changes have already tried to address bugs
>when the system time is changed but some error conditions still exist,
>because the logic is still coupled with the wall time.
>
>If the time is changed in between the SA is created and the tasklet timer
>is started for the first time, the SA scheduling will be broken:
>either the SA will expire and never be recreated, or it will expire at
>an unexpected time.  The reason is that x->curlft.add_time will not be valid
>when the "next" variable is computed for the very first time
>in xfrm_timer_handler().
>
>Fix this behaviour by avoiding to rely on the system time.
>Stick to relative time intervals and realise a total decoupling
>from the wall time.
>
>Based on another patch written and published by
>Fan Du (fan.du@...el.com) in 2013 but never merged:
>part of the code preserved, some rewritten and improved.
>Changes to the logic accounting for the use_time expiration.
>Here we allow both add_time and use_time expirations to be set.
>
>Cc: Steffen Klassert <steffen.klassert@...unet.com>
>Cc: David S. Miller <davem@...emloft.net>
>Cc: Fan Du <fan.du@...el.com>
>Cc: Alexander Sverdlin <alexander.sverdlin@...ia.com>
>Cc: Matija Glavinic Pecotic <matija.glavinic-pecotic.ext@...ia.com>
>Signed-off-by: Giuseppe Cantavenera <giuseppe.cantavenera.ext@...ia.com>
>Signed-off-by: Nicholas Faustini <nicholas.faustini.ext@...ia.com>
>---
>
>Hello,
>
>we also meet the same bug Fan Du did a while ago.
>Two solutions were proposed in the past:
>either forcibly mark as expired all of the keys every time the clock is set,
>or replace the existing timers with relative ones.
>
>The former would introduce unexpected behaviour
>(the keys would keep expiring when they shouldn't) and does not address the
>real problem: THE COUPLING between the SA scheduling and the wall timer.
>Actually it introduces even more of that.
>
>The latter is robust, extremly lightweight and maintanable, and preserves the
>expected behaviour, that's why we preferred it.
>
>Any feedback or any other idea is greatly appreciated.

Thanks for keep working this issue as I did 2 years ago.

Objection against the original approach from the maintainers is that it complicates
the logic to the degree which involving extra maintenance effort, that is the effort
it's not worthwhile against the trouble it might introduce in the future.

Another approach you can try is using monotonic boot time(counting in suspend time also)
to mark the life time of SA, then the timer handler logic will be quite easier and smaller
than now, sure it will be robust naturally. The cost is that SA lifetime displaying by setkey
and SA migration has to be taken care of as SA life time is boot time now, not the wall time.


>Thanks,
>Regards,
>Giuseppe
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html