Re: [PATCH] landlock: shrink tsync works[] on partial allocation failure

["Günther Noack" <gnoack@google.com>]

Hello Peng!

Thanks for your patch!

On Tue, Jun 23, 2026 at 01:01:27PM +0800, Peng Hao wrote:
> When the per-slot kzalloc fails mid-loop in tsync_works_grow_by(), the
> already-enlarged s->works array keeps uninitialized trailing entries.
> Shrink the array back to its used size on the error path so no waste
> is carried over: free it outright when nothing has been allocated yet,
> otherwise try a shrinking krealloc_array() (keep the larger array if
> the shrink fails, since tsync_works_release() honors s->capacity).
> 
> Signed-off-by: Peng Hao <flyingpeng@tencent.com>
> ---
>  security/landlock/tsync.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> index c5730bbd..356ce94b 100644
> --- a/security/landlock/tsync.c
> +++ b/security/landlock/tsync.c
> @@ -272,9 +272,23 @@ static int tsync_works_grow_by(struct tsync_works *s, size_t n, gfp_t flags)
>  		work = kzalloc_obj(*work, flags);
>  		if (!work) {
>  			/*
> -			 * Leave the object in a consistent state,
> -			 * but return an error.
> +			 * Leave the object in a consistent state, but return
> +			 * an error.  Shrink @s->works back to its used size to
> +			 * avoid carrying uninitialized trailing entries.  A
> +			 * shrinking krealloc_array() should normally succeed,
> +			 * but if it does not we simply keep the larger array;
> +			 * tsync_works_release() iterates only up to capacity.
>  			 */
> +			if (i == 0) {
> +				kfree(s->works);
> +				s->works = NULL;
> +			} else {
> +				works = krealloc_array(s->works, i,
> +						       sizeof(s->works[0]),
> +						       flags | __GFP_NOWARN);
> +				if (works)
> +					s->works = works;
> +			}
>  			s->capacity = i;
>  			return -ENOMEM;
>  		}

Can you please clarify your motivation for this?

To paraphrase my understanding

* You are not addressing a logic bug

  The invariant for that data structure is that s->size <= s->capacity
  and s->capacity <= number of elements in the array <= number of
  sibling threads.

  If the array is slightly larger than the capacity, that does not break
  the invariant and should not result in out-of-bounds accesses.

* You are addressing that the array is a bit larger than the capacity

  This is in the case where kzalloc_obj() failed.  We set the capacity
  to i (making sure that only the objects 0 to i-1 are being looked at),
  and we return an error.  Sure, the array is overallocated a little
  bit, but now that we are returning an error, the caller will
  fast-track to abort the tsync due to ENOMEM and the array does anyway
  get released very soon now.


The state where we use slightly more memory (with number of entries <=
number of sibling threads) is supposed to be very transient.

Is the delay between raising the error and the final kfree() long enough
that you have seen it cause problems in practice?

Thanks,
—Günther