On 2/13/19 5:12 AM, Petr Mladek wrote:
My idea is to create something similar to the shadow variables API.
Something like:
struct klp_feature {
/* feature ID, e.g. #define L1TF 1 */
int id;
/* Version of the code */
int version;
/* object + callbacks assiciated with the feature */
struct klp_object *obj;
/* optional data */
void *data;
/* list of available features */
struct list_head node;
*/
>
Note that it would be possible to add id, version, data to
struct klp_callbacks.
But more practical would be to create it dynamically.
Then one callback might support more features. Also
It is less error-prone for porting because everyhing
is self-contained in the callback code. Finally,
it makes it easier to take over the state by another
cummulative livepatch.
Now the callbacks:
struct klp_feature *klp_feature_alloc(id, version, obj, data);
void klp_feature_free(feature);
struct klp_feature *klp_feature_get(id);
struct klp_feature *klp_feature_get(id);
Then the callback might do something like:
#define L1TF 1
#define L1TF_VERSION 1
int klp_pre_patch_callback_l1tf(struct klp_object *obj)
{
struct klp_feature *feature;
if (klp_feature_get(L1TF))
return 0;
/* this is the first patch doing L1TF */
feature = klp_feature_alloc(L1TF, L1TF_VERSION, obj, NULL);
if (!feature)
return -ENOMEM;
}
void klp_post_callback_l1tf(struct klp_object *obj)
{
struct klp_feature *feature;
feature = klp_get_feature(L1TF);
if (!feature) {
WARN("This should never happen\n);
if (feature->obj != obj && old_feature->version == L1TF_VERSION)
{
/* just take over */
old_feature->obj = obj;
return;
}
# enable L1TF mitigation
l1tf_enable();
}
void klp_pre_unpatch_callback(struct klp_object *obj)
{
/* Disable l1tf mitigating, revert all changes */
l1tf_disable();
}
void klp_post_unpatch_callback(struct klp_object *obj)
{
struct klp_feature *feature;
feature = klp_get_feature(L1TF);
if (!feature) {
WARN("This should never happen\n);
if (feature->obj != obj && feature->version == L1TF_VERSION) {
/* This is revert, previous patch keeps using it */
return;
}
klp_feature_free(feature);
}
Finally, the question is how to check the compatibility within update.
In this case, it might be easier if version, id is part of
struct klp_callbacks.
Alternative solution would be to add another callback:
int klp_callback_feature_version(id, version);
Then we could do:
/* all features supported */
bool compatible = true;
for_each_allocated_feature(feature) {
bool found_compat = false;
for_each_object() {
int version;
version = obj->callbacks->feature_version(feature->id);
if (version >= feature->version) {
found_compat == true;
break;
}
}
}
if (!found_compat) {
compatible = false;
break;
}
It is more complicated than the global version. But it is not
a rocket since either.
Now, the question is how much you like it.
I see where this is going and off the top of my head, I think it would
address the L1TF upgrade/downgrade safety concerns.
If you prefer, we can revist this when there is code to review, but here
my initial questions and impressions:
- Do features need to be tightly coupled to callbacks? They are a
convenient tool to implement a policy, but I wonder if we lose anything
by moving this up to the klp_patch level and letting the core handle it.
(Like the versioning scheme.) Features could still be stored in a
list and the callbacks could get access to current vs. this patch's
feature set. They wouldn't be dynamic, but then the API supporting that
could drop out.
- Perhaps the features-to-callback connection in my mind is too strong.
It makes it sound like the callbacks themselves have provided a feature.
It is possible that patched routines are the unsafe culprits while
callbacks only a mechanism to enforce safety. I guess this is why I
think to abstract features out to the patch level.
- One more point on the abstraction: pulling this out of the callbacks
may allow one to understand livepatch features without the details of
the callbacks. Not sure if that's a good or bad thing ...
- For follow on livepatches, would their feature set need to match or
exceed the values for all of the current features, or only the features
that the new patch is advertising to support? For example, if we have
current features:
vmlinux : { { L1TF, v1 }, { CVE2, v2 } }
should the incoming cumulative patch need L1TF >= v1 and CVE2 >= v2 ?
Or does it only need L1TF >= v1 *or* CVE2 >= v2?
Dunno if cumulative livepatches change that requirement or not.
- Wacky idea: can features be deprecated? A livepatch neuters the
unsafe condition, so future patches can now safely ignore it.
Sorry for all the thinking out loud, but I figured I would share my
frame of mind after coming out of the version/sticky RFC code. tl;dr: I
think this would technically work, the question is how elaborate a
policy do we need and then where does it need to be implemented.
-- Joe
