Hi all, Joe Lawrence <joe.lawrence@xxxxxxxxxx> writes: > On Sun, Jul 14, 2024 at 09:59:32PM +0200, raschupkin.ri@xxxxxxxxx wrote: >> > But first, let me see if I understand the problem correctly. Let's say > points A and A' below represent the original kernel code reference > get/put pairing in task execution flow. A livepatch adds a new get/put > pair, B and B' in the middle like so: > > --- execution flow ---> > -- A B B' A' --> > > There are potential issues if the livepatch is (de)activated > mid-sequence, between the new pairings: > > problem 1: > -- A . B' A' --> 'B, but no B = extra put! > ^ livepatch is activated here > > problem 2: > -- A B . A' --> B, but no B' = extra get! > ^ livepatch is deactivated here I can confirm that this scenario happens quite often with real world CVE fixes and there's currently no way to implement such changes safely from a livepatch. But I also believe this is an instance of a broader problem class we attempted to solve with that "enhanced" states API proposed and discussed at LPC ([1], there's a link to a recording at the bottom). For reference, see Petr's POC from [2]. > The first thing that comes to mind is that this might be solved using > the existing shadow variable API. Same. > When the livepatch takes the new > reference (B), it could create a new <struct, NEW_REF> shadow variable > instance. The livepatch code to return the reference (B') would then > check on the shadow variable existence before doing so. This would > solve problem 1. > > The second problem is a little trickier. Perhaps the shadow variable > approach still works as long as a pre-unpatch hook* were to iterate > through all the <*, NEW_REF> shadow variable instances and returned > their reference before freeing the shadow variable and declaring the > livepatch inactive. I think the problem of consistently maintaining shadowed reference counts (or anything shadowed for that matter) could be solved with the help of aforementioned states API enhancements, so I would propose to revive Petr's IMO more generic patchset as an alternative. Thoughts? Thanks, Nicolai [1] https://lpc.events/event/17/contributions/1541/ [2] https://lore.kernel.org/r/20231110170428.6664-1-pmladek@xxxxxxxx -- SUSE Software Solutions Germany GmbH, Frankenstraße 146, 90461 Nürnberg, Germany GF: Ivo Totev, Andrew McDonald, Werner Knoblich (HRB 36809, AG Nürnberg)
