All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Rosato <mjrosato@linux.ibm.com>
To: Farhan Ali <alifm@linux.ibm.com>,
	linux-kernel@vger.kernel.org, linux-s390@vger.kernel.org,
	kvm@vger.kernel.org
Cc: borntraeger@linux.ibm.com, farman@linux.ibm.com
Subject: Re: [PATCH v4 2/6] KVM: s390: pci: Fix memory accounting for pinned/unpinned pages
Date: Thu, 23 Jul 2026 10:12:09 -0400	[thread overview]
Message-ID: <6f87bda7-8a8a-4f93-907c-6e0a442c8ceb@linux.ibm.com> (raw)
In-Reply-To: <20260722170621.1686-3-alifm@linux.ibm.com>

On 7/22/26 1:06 PM, Farhan Ali wrote:
> The account_mem() and unaccount_mem() functions call get_uid() which
> increments the reference count of struct user_struct on every invocation.
> But we don't decrement the count by calling free_uid(). It also
> accounted/unaccounted the pages against the current->mm. But its possible
> the unaccount_mem() can be called from a different process context than the
> one that originally pinned the pages.
> 
> Let's fix this by storing the pinning process user_struct and mm_struct
> when accounting for pinned pages, and subsequently free these resources
> when the pages are unpinned.
> 
> Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
> ---
>  arch/s390/kvm/pci.c | 38 ++++++++++++++++++++++++++++----------
>  arch/s390/kvm/pci.h |  2 ++
>  2 files changed, 30 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index d2a11cdf6941..1b3114c7cfbb 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c
> @@ -190,33 +190,51 @@ static int kvm_zpci_clear_airq(struct zpci_dev *zdev)
>  	return cc ? -EIO : 0;
>  }
>  
> -static inline void unaccount_mem(unsigned long nr_pages)
> +static inline void unaccount_mem(struct kvm_zdev *kzdev, unsigned long nr_pages)
>  {
> -	struct user_struct *user = get_uid(current_user());
> +	struct user_struct *user = kzdev->user_account;
> +	struct mm_struct *mm_account = kzdev->mm_account;
>  
> -	if (user)
> +	if (user) {
>  		atomic_long_sub(nr_pages, &user->locked_vm);
> -	if (current->mm)
> -		atomic64_sub(nr_pages, &current->mm->pinned_vm);

Previous code handled the case where current->mm could be NULL...

> +		free_uid(user);
> +		kzdev->user_account = NULL;
> +	}
> +
> +	if (mm_account) {

... And you check for kzdev->mm_account being NULL here...

> +		atomic64_sub(nr_pages, &mm_account->pinned_vm);
> +		mmdrop(mm_account);
> +		kzdev->mm_account = NULL;
> +	}
>  }
>  
> -static inline int account_mem(unsigned long nr_pages)
> +static inline int account_mem(struct kvm_zdev *kzdev, unsigned long nr_pages)
>  {
>  	struct user_struct *user = get_uid(current_user());
>  	unsigned long page_limit, cur_pages, new_pages;
> +	int rc = 0;
>  
>  	page_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
>  
>  	cur_pages = atomic_long_read(&user->locked_vm);
>  	do {
>  		new_pages = cur_pages + nr_pages;
> -		if (new_pages > page_limit)
> -			return -ENOMEM;
> +		if (new_pages > page_limit) {
> +			rc =  -ENOMEM;
> +			goto out;
> +		}
>  	} while (!atomic_long_try_cmpxchg(&user->locked_vm, &cur_pages, new_pages));
>  
> +	mmgrab(current->mm);

... But here you do not check if current->mm is NULL.  I'm not sure if
it will happen in practice, but we did guard against it before this
patch.  Just add if (!current->mm) around this mmgrab?

>  	atomic64_add(nr_pages, &current->mm->pinned_vm);
> +	kzdev->user_account = user;
> +	kzdev->mm_account = current->mm;

Then if it IS NULL, we will stash a NULL into kzdev->mm_account here and
handle it above as before with the if (mm_account) check.

>  
>  	return 0;
> +
> +out:
> +	free_uid(user);
> +	return rc;
>  }
>  
>  static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
> @@ -279,7 +297,7 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>  	}
>  
>  	/* Account for pinned pages, roll back on failure */
> -	if (account_mem(pcount))
> +	if (account_mem(zdev->kzdev, pcount))
>  		goto unpin2;
>  
>  	/* AISB must be allocated before we can fill in GAITE */
> @@ -400,7 +418,7 @@ static int kvm_s390_pci_aif_disable(struct zpci_dev *zdev, bool force)
>  		pcount++;
>  	}
>  	if (pcount > 0)
> -		unaccount_mem(pcount);
> +		unaccount_mem(kzdev, pcount);
>  out:
>  	mutex_unlock(&aift->aift_lock);
>  
> diff --git a/arch/s390/kvm/pci.h b/arch/s390/kvm/pci.h
> index ff0972dd5e71..544e6aa75e38 100644
> --- a/arch/s390/kvm/pci.h
> +++ b/arch/s390/kvm/pci.h
> @@ -21,6 +21,8 @@ struct kvm_zdev {
>  	struct zpci_dev *zdev;
>  	struct kvm *kvm;
>  	struct zpci_fib fib;
> +	struct user_struct *user_account;
> +	struct mm_struct *mm_account;
>  	struct list_head entry;
>  };
>  


  parent reply	other threads:[~2026-07-23 14:12 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-22 17:06 [PATCH v4 0/6] KVM s390x PCI fixes Farhan Ali
2026-07-22 17:06 ` [PATCH v4 1/6] KVM: s390: pci: Reject adapter interrupt forwarding if already enabled Farhan Ali
2026-07-22 17:25   ` sashiko-bot
2026-07-23  6:57   ` Christian Borntraeger
2026-07-23 14:11   ` Matthew Rosato
2026-07-22 17:06 ` [PATCH v4 2/6] KVM: s390: pci: Fix memory accounting for pinned/unpinned pages Farhan Ali
2026-07-22 17:21   ` sashiko-bot
2026-07-23 12:08   ` Christian Borntraeger
2026-07-23 14:12   ` Matthew Rosato [this message]
2026-07-23 17:08     ` Farhan Ali
2026-07-23 17:31       ` Matthew Rosato
2026-07-22 17:06 ` [PATCH v4 3/6] KVM: s390: pci: Fix missing error codes and memory unaccounting Farhan Ali
2026-07-22 17:17   ` sashiko-bot
2026-07-23  7:34   ` Christian Borntraeger
2026-07-23 14:15   ` Matthew Rosato
2026-07-22 17:06 ` [PATCH v4 4/6] KVM: s390: pci: Fix NULL dereference on AIBV allocation failure Farhan Ali
2026-07-22 17:19   ` sashiko-bot
2026-07-23  8:18   ` Christian Borntraeger
2026-07-23 14:20   ` Matthew Rosato
2026-07-22 17:06 ` [PATCH v4 5/6] KVM: s390: pci: Fix resource leak on IRQ registration failure Farhan Ali
2026-07-22 17:15   ` sashiko-bot
2026-07-23 12:17   ` Christian Borntraeger
2026-07-23 15:19   ` Matthew Rosato
2026-07-23 16:55     ` Farhan Ali
2026-07-22 17:06 ` [PATCH v4 6/6] KVM: s390: pci: Validate AIBV and AISB before pinning guest pages Farhan Ali
2026-07-22 17:26   ` sashiko-bot
2026-07-23 11:57     ` Christian Borntraeger
2026-07-23 16:44       ` Farhan Ali
2026-07-23 12:19   ` Christian Borntraeger
2026-07-23 15:35   ` Matthew Rosato
2026-07-23 12:59 ` [PATCH v4 0/6] KVM s390x PCI fixes Christian Borntraeger
2026-07-23 13:00   ` Matthew Rosato
2026-07-23 15:35     ` Matthew Rosato
2026-07-23 16:38       ` Farhan Ali

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=6f87bda7-8a8a-4f93-907c-6e0a442c8ceb@linux.ibm.com \
    --to=mjrosato@linux.ibm.com \
    --cc=alifm@linux.ibm.com \
    --cc=borntraeger@linux.ibm.com \
    --cc=farman@linux.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.