[PATCH] fsverity: Fix bpf_get_fsverity_digest() dynptr assumptions

Eric Biggers posted 1 patch 2 months ago
fs/verity/measure.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
[PATCH] fsverity: Fix bpf_get_fsverity_digest() dynptr assumptions
Posted by Eric Biggers 2 months ago
The BPF verifier and the dynptr abstraction ensure that the memory space
referenced by a dynptr remains valid.  They do not, however, provide any
guarantee that the contents of the memory are stable.  kfuncs are
expected to remain memory-safe even if concurrent modifications occur.

bpf_get_fsverity_digest() didn't follow that: it could crash if
arg->digest_size was concurrently modified.

Fix that by using the known-good value hash_alg->digest_size instead.

Also correctly handle sizes over INT_MAX, which previously caused an
integer overflow and crash.  __bpf_dynptr_size() returns a u64.

Fixes: 67814c00de31 ("bpf, fsverity: Add kfunc bpf_get_fsverity_digest")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 fs/verity/measure.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/verity/measure.c b/fs/verity/measure.c
index cfe2d5e535f9..f8b3526af004 100644
--- a/fs/verity/measure.c
+++ b/fs/verity/measure.c
@@ -122,11 +122,11 @@ __bpf_kfunc int bpf_get_fsverity_digest(struct file *file, const struct bpf_dynp
 {
 	const struct bpf_dynptr_kern *digest_ptr = (struct bpf_dynptr_kern *)digest_p;
 	const struct inode *inode = file_inode(file);
-	u32 dynptr_sz = __bpf_dynptr_size(digest_ptr);
+	u64 dynptr_sz = __bpf_dynptr_size(digest_ptr);
 	struct fsverity_digest *arg;
 	const struct fsverity_info *vi;
 	const struct fsverity_hash_alg *hash_alg;
-	int out_digest_sz;
+	u64 out_digest_sz;
 
 	if (dynptr_sz < sizeof(struct fsverity_digest))
 		return -EINVAL;
@@ -150,11 +150,13 @@ __bpf_kfunc int bpf_get_fsverity_digest(struct file *file, const struct bpf_dynp
 	out_digest_sz = dynptr_sz - sizeof(struct fsverity_digest);
 
 	/* copy digest */
-	memcpy(arg->digest, vi->file_digest,  min_t(int, hash_alg->digest_size, out_digest_sz));
+	memcpy(arg->digest, vi->file_digest,
+	       min(hash_alg->digest_size, out_digest_sz));
 
 	/* fill the extra buffer with zeros */
 	if (out_digest_sz > hash_alg->digest_size)
-		memset(arg->digest + arg->digest_size, 0, out_digest_sz - hash_alg->digest_size);
+		memset(arg->digest + hash_alg->digest_size, 0,
+		       out_digest_sz - hash_alg->digest_size);
 
 	return 0;
 }

base-commit: f5098b6bae761e346ebcd9da7f95622c04733cff
-- 
2.55.0.508.g3f0d502094-goog
Re: [PATCH] fsverity: Fix bpf_get_fsverity_digest() dynptr assumptions
Posted by Song Liu 2 months ago
On Thu, Jul 30, 2026 at 5:43 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> The BPF verifier and the dynptr abstraction ensure that the memory space
> referenced by a dynptr remains valid.  They do not, however, provide any
> guarantee that the contents of the memory are stable.  kfuncs are
> expected to remain memory-safe even if concurrent modifications occur.
>
> bpf_get_fsverity_digest() didn't follow that: it could crash if
> arg->digest_size was concurrently modified.
>
> Fix that by using the known-good value hash_alg->digest_size instead.
>
> Also correctly handle sizes over INT_MAX, which previously caused an
> integer overflow and crash.  __bpf_dynptr_size() returns a u64.
>
> Fixes: 67814c00de31 ("bpf, fsverity: Add kfunc bpf_get_fsverity_digest")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>

Looks good to me. Thanks for the fix!

Acked-by: Song Liu <song@kernel.org>
Re: [PATCH] fsverity: Fix bpf_get_fsverity_digest() dynptr assumptions
Posted by Kumar Kartikeya Dwivedi 2 months ago
On Fri Jul 31, 2026 at 2:39 AM CEST, Eric Biggers wrote:
> The BPF verifier and the dynptr abstraction ensure that the memory space
> referenced by a dynptr remains valid.  They do not, however, provide any
> guarantee that the contents of the memory are stable.  kfuncs are
> expected to remain memory-safe even if concurrent modifications occur.
>
> bpf_get_fsverity_digest() didn't follow that: it could crash if
> arg->digest_size was concurrently modified.
>
> Fix that by using the known-good value hash_alg->digest_size instead.
>

This makes sense.

> Also correctly handle sizes over INT_MAX, which previously caused an
> integer overflow and crash.  __bpf_dynptr_size() returns a u64.
>

I guess this occurs when using file dynptrs? Otherwise the size is capped to
DYNPTR_SIZE_MASK.

> Fixes: 67814c00de31 ("bpf, fsverity: Add kfunc bpf_get_fsverity_digest")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> ---

Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>

> [...]
Re: [PATCH] fsverity: Fix bpf_get_fsverity_digest() dynptr assumptions
Posted by Eric Biggers 2 months ago
On Sat, Aug 01, 2026 at 06:52:45PM +0200, Kumar Kartikeya Dwivedi wrote:
> > Also correctly handle sizes over INT_MAX, which previously caused an
> > integer overflow and crash.  __bpf_dynptr_size() returns a u64.
> >
> 
> I guess this occurs when using file dynptrs? Otherwise the size is capped to
> DYNPTR_SIZE_MASK.

Yes, though it looks like __bpf_dynptr_data_rw() excludes file dynptrs.
So this particular issue might be unreachable.  Still seems like a good
idea to match the u64 type that __bpf_dynptr_size() returns though.

- Eric
Re: [PATCH] fsverity: Fix bpf_get_fsverity_digest() dynptr assumptions
Posted by Kumar Kartikeya Dwivedi 2 months ago
On Sat Aug 1, 2026 at 7:15 PM CEST, Eric Biggers wrote:
> On Sat, Aug 01, 2026 at 06:52:45PM +0200, Kumar Kartikeya Dwivedi wrote:
>> > Also correctly handle sizes over INT_MAX, which previously caused an
>> > integer overflow and crash.  __bpf_dynptr_size() returns a u64.
>> >
>>
>> I guess this occurs when using file dynptrs? Otherwise the size is capped to
>> DYNPTR_SIZE_MASK.
>
> Yes, though it looks like __bpf_dynptr_data_rw() excludes file dynptrs.
> So this particular issue might be unreachable.  Still seems like a good
> idea to match the u64 type that __bpf_dynptr_size() returns though.
>

I don't see this in patchwork, so you might have to resend this with [PATCH
bpf-next] subject again to trigger CI if you want this to go through bpf tree.

> - Eric
Re: [PATCH] fsverity: Fix bpf_get_fsverity_digest() dynptr assumptions
Posted by Kumar Kartikeya Dwivedi 2 months ago
On Sat Aug 1, 2026 at 7:35 PM CEST, Kumar Kartikeya Dwivedi wrote:
> On Sat Aug 1, 2026 at 7:15 PM CEST, Eric Biggers wrote:
>> On Sat, Aug 01, 2026 at 06:52:45PM +0200, Kumar Kartikeya Dwivedi wrote:
>>> > Also correctly handle sizes over INT_MAX, which previously caused an
>>> > integer overflow and crash.  __bpf_dynptr_size() returns a u64.
>>> >
>>>
>>> I guess this occurs when using file dynptrs? Otherwise the size is capped to
>>> DYNPTR_SIZE_MASK.
>>
>> Yes, though it looks like __bpf_dynptr_data_rw() excludes file dynptrs.
>> So this particular issue might be unreachable.  Still seems like a good
>> idea to match the u64 type that __bpf_dynptr_size() returns though.
>>
>
> I don't see this in patchwork, so you might have to resend this with [PATCH
> bpf-next] subject again to trigger CI if you want this to go through bpf tree.
>

Sorry, [PATCH bpf], rather.

>> - Eric
Re: [PATCH] fsverity: Fix bpf_get_fsverity_digest() dynptr assumptions
Posted by Eric Biggers 2 months ago
On Sat, Aug 01, 2026 at 07:38:08PM +0200, Kumar Kartikeya Dwivedi wrote:
> On Sat Aug 1, 2026 at 7:35 PM CEST, Kumar Kartikeya Dwivedi wrote:
> > On Sat Aug 1, 2026 at 7:15 PM CEST, Eric Biggers wrote:
> >> On Sat, Aug 01, 2026 at 06:52:45PM +0200, Kumar Kartikeya Dwivedi wrote:
> >>> > Also correctly handle sizes over INT_MAX, which previously caused an
> >>> > integer overflow and crash.  __bpf_dynptr_size() returns a u64.
> >>> >
> >>>
> >>> I guess this occurs when using file dynptrs? Otherwise the size is capped to
> >>> DYNPTR_SIZE_MASK.
> >>
> >> Yes, though it looks like __bpf_dynptr_data_rw() excludes file dynptrs.
> >> So this particular issue might be unreachable.  Still seems like a good
> >> idea to match the u64 type that __bpf_dynptr_size() returns though.
> >>
> >
> > I don't see this in patchwork, so you might have to resend this with [PATCH
> > bpf-next] subject again to trigger CI if you want this to go through bpf tree.
> >
> 
> Sorry, [PATCH bpf], rather.

Sure I can do that.

Would it be okay to also make this function return -EOVERFLOW if
out_digest_sz is too small, similar to FS_IOC_MEASURE_VERITY, instead of
silently truncating?  The silent truncation is a footgun.

- Eric
Re: [PATCH] fsverity: Fix bpf_get_fsverity_digest() dynptr assumptions
Posted by Kumar Kartikeya Dwivedi 2 months ago
On Sat, 1 Aug 2026 at 22:18, Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Sat, Aug 01, 2026 at 07:38:08PM +0200, Kumar Kartikeya Dwivedi wrote:
> > On Sat Aug 1, 2026 at 7:35 PM CEST, Kumar Kartikeya Dwivedi wrote:
> > > On Sat Aug 1, 2026 at 7:15 PM CEST, Eric Biggers wrote:
> > >> On Sat, Aug 01, 2026 at 06:52:45PM +0200, Kumar Kartikeya Dwivedi wrote:
> > >>> > Also correctly handle sizes over INT_MAX, which previously caused an
> > >>> > integer overflow and crash.  __bpf_dynptr_size() returns a u64.
> > >>> >
> > >>>
> > >>> I guess this occurs when using file dynptrs? Otherwise the size is capped to
> > >>> DYNPTR_SIZE_MASK.
> > >>
> > >> Yes, though it looks like __bpf_dynptr_data_rw() excludes file dynptrs.
> > >> So this particular issue might be unreachable.  Still seems like a good
> > >> idea to match the u64 type that __bpf_dynptr_size() returns though.
> > >>
> > >
> > > I don't see this in patchwork, so you might have to resend this with [PATCH
> > > bpf-next] subject again to trigger CI if you want this to go through bpf tree.
> > >
> >
> > Sorry, [PATCH bpf], rather.
>
> Sure I can do that.
>
> Would it be okay to also make this function return -EOVERFLOW if
> out_digest_sz is too small, similar to FS_IOC_MEASURE_VERITY, instead of
> silently truncating?  The silent truncation is a footgun.

Yeah, I think it makes sense. I will also ping Song to ack this on
Monday (or your new version).
Thanks

>
> - Eric