Re: [PATCH v4 14/19] liveupdate: Add feature header to struct luo_flb_ser

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] liveupdate: -EOPNOTSUPP collision in luo_flb_retrieve_one()
  on validation
--

commit f2fc7f3d05b147a7eeaad5c4521b29383721337e
Author: Tarun Sahu <tarunsahu@google.com>

liveupdate: Add feature header to struct luo_flb_ser

This commit adds a struct kho_ser_hdr feature header to each serialized
struct luo_flb_ser entry and validates each entry header during retrieval
in luo_flb_retrieve_one().

> diff --git a/kernel/liveupdate/luo_flb.c b/kernel/liveupdate/luo_flb.c
> index a8dcdb8456fd2..d38c223741ff4 100644
> --- a/kernel/liveupdate/luo_flb.c
> +++ b/kernel/liveupdate/luo_flb.c
[ ... ]
> @@ -177,10 +178,18 @@ static int luo_flb_retrieve_one(struct liveupdate_flb *flb)
>  	if (!fh->active)
>  		return -ENODATA;
>  
> -	for (int i = 0; i < fh->header_ser->count; i++) {
> -		if (!strcmp(fh->ser[i].name, flb->compatible)) {
> -			private->incoming.data = fh->ser[i].data;
> -			refcount_set(&private->incoming.count, fh->ser[i].count);
> +	for (u64 i = 0; i < fh->header_ser->count;
> +	     i++, fhdr = (void *)fhdr + fhdr->size) {
> +		struct luo_flb_ser *ser;
> +
> +		err = kho_ser_validate(fhdr, luo_flb_ser);
> +		if (err)
> +			return err;
> +
> +		ser = kho_ser_payload(fhdr, luo_flb_ser);
> +		if (!strcmp(ser->name, flb->compatible)) {
> +			private->incoming.data = ser->data;
> +			refcount_set(&private->incoming.count, ser->count);
>  			found = true;
>  			break;
>  		}

[Severity: Medium]
Does returning -EOPNOTSUPP from kho_ser_validate() collide with the
-EOPNOTSUPP return value that liveupdate_flb_get_incoming() uses to
indicate that Live Update is disabled?

When an incoming struct luo_flb_ser entry has an unsupported required
feature bit set, kho_ser_validate() returns -EOPNOTSUPP, which
liveupdate_flb_get_incoming() propagates to callers:

drivers/pci/liveupdate.c:pci_liveupdate_flb_get_incoming() {
	...
	ret = liveupdate_flb_get_incoming(&pci_liveupdate_flb, (void **)&incoming);

	/* Live Update is not enabled. */
	if (ret == -EOPNOTSUPP)
		return NULL;
	...
}

Instead of panicking on incompatible incoming state, the PCI subsystem
treats Live Update as disabled and proceeds with normal enumeration over
preserved devices.

Also, kho_ser_validate() runs on every entry before checking ser->name, so
an unsupported required feature on an earlier entry fails lookups for later
compatible FLBs, and returning before refcount_set(&private->incoming.count,
ser->count) leaves private->incoming.count at 0, which underflows in
liveupdate_flb_put_incoming() during luo_file_finish().  Validating all
luo_flb_ser entries up front in luo_flb_setup_incoming() (or translating
the validation error code) would avoid both issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110050202.529473-1-tarunsahu@google.com?part=14