public inbox for op-tee@lists.trustedfirmware.org
 help / color / mirror / Atom feed
From: Sumit Garg via OP-TEE <op-tee@lists.trustedfirmware.org>
To: Jens Wiklander <jens.wiklander@linaro.org>,
	Masami Ichikawa <masami256@gmail.com>
Cc: op-tee@lists.trustedfirmware.org
Subject: Re: [BUG] tee_shm: NULL pointer dereference in unpin_user_pages() on invalid shm pages
Date: Fri, 19 Sep 2025 11:35:51 +0530	[thread overview]
Message-ID: <aMzyv0BKFOiE9Jgm@sumit-X1> (raw)
In-Reply-To: <20250918122541.GA2693176@rayden>

On Thu, Sep 18, 2025 at 02:25:41PM +0200, Jens Wiklander wrote:
> Hi Masami,
> 
> [+Sumit in CC]
> 
> On Wed, Sep 17, 2025 at 10:58:11PM +0900, Masami Ichikawa wrote:
> [snip]
> > I wrote a test program and ran it on both 6.17-rc5 and 6.14. I was
> > able to reproduce the crash on both kernels.
> > 
> > I uploaded test code and test results to my gist.
> > https://gist.github.com/masami256/11e21a7503812af7ee1e890080093a2c
> > 
> > The test code is crash_test.c. This program takes 2 arguments. First
> > argument is malicious buffer size and second one is actual buffer
> > size.
> > I can reproduce the crash with the following pair.
> > 
> > malicious buffer size: 0xffffff
> > actual buffer size: 0xff
> 

Thanks Masami for the report and the bug reproducer here.

> Thanks, that easily reproduces the problem. The following diff should fix it:
> --- a/drivers/tee/tee_shm.c
> +++ b/drivers/tee/tee_shm.c
> @@ -318,7 +318,16 @@ register_shm_helper(struct tee_context *ctx, struct iov_iter *iter, u32 flags,
>  
>  	len = iov_iter_extract_pages(iter, &shm->pages, LONG_MAX, num_pages, 0,
>  				     &off);
> -	if (unlikely(len <= 0)) {
> +	if (unlikely(len < num_pages * PAGE_SIZE)) {
> +		if (len > 0) {
> +			/*
> +			 * If we only got a few pages, update to release
> +			 * the correct amount below.
> +			 */
> +			shm->num_pages = len / PAGE_SIZE;
> +			ret = ERR_PTR(-ENOMEM);
> +			goto err_put_shm_pages;
> +		}
>  		ret = len ? ERR_PTR(len) : ERR_PTR(-ENOMEM);
>  		goto err_free_shm_pages;
>  	}

Thanks Jens for the fix, it sounds appropriate to me. I think this
commit [1] introduced the bug in the first place as earlier check for
pin_user_pages_fast() would have caught this issue without crashing the
kernel.

Jens, can you please send a proper fix here? I hope we should be able to
get it merged for v6.17 since it sounds critical to me.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7bdee41575919773818e525ea19e54eb817770af

-Sumit

      parent reply	other threads:[~2025-09-19  6:06 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-13  6:06 [BUG] tee_shm: NULL pointer dereference in unpin_user_pages() on invalid shm pages Masami Ichikawa
2025-09-16  7:46 ` Jens Wiklander
2025-09-17 13:58   ` Masami Ichikawa
2025-09-18 12:25     ` Jens Wiklander
2025-09-18 14:31       ` Masami Ichikawa
2025-09-19  6:05       ` Sumit Garg via OP-TEE [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aMzyv0BKFOiE9Jgm@sumit-X1 \
    --to=op-tee@lists.trustedfirmware.org \
    --cc=jens.wiklander@linaro.org \
    --cc=masami256@gmail.com \
    --cc=sumit.garg@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox