Linux-NFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] sunrpc: expiry_time should be seconds not timeval
@ 2020-02-04 10:32 Roberto Bergantinos Corpas
  0 siblings, 0 replies; 4+ messages in thread
From: Roberto Bergantinos Corpas @ 2020-02-04 10:32 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs, arnd

When upcalling gssproxy, cache_head.expiry_time is set as a
timeval, not seconds since boot. As such, RPC cache expiry
logic will not clean expired objects created under
auth.rpcsec.context cache.

This has proven to cause kernel memory leaks on field. Using
64 bit variants of getboottime/timespec

Signed-off-by: Roberto Bergantinos Corpas <rbergant@redhat.com>
---
 net/sunrpc/auth_gss/svcauth_gss.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 7511a68aadf0..65b67b257302 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -1248,6 +1248,7 @@ static int gss_proxy_save_rsc(struct cache_detail *cd,
 		dprintk("RPC:       No creds found!\n");
 		goto out;
 	} else {
+		struct timespec64 boot;
 
 		/* steal creds */
 		rsci.cred = ud->creds;
@@ -1268,6 +1269,9 @@ static int gss_proxy_save_rsc(struct cache_detail *cd,
 						&expiry, GFP_KERNEL);
 		if (status)
 			goto out;
+
+		getboottime64(&boot);
+		expiry -= boot.tv_sec;
 	}
 
 	rsci.h.expiry_time = expiry;
-- 
2.21.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] sunrpc: expiry_time should be seconds not timeval
  2020-01-24 16:53 ` Frank Sorenson
@ 2020-01-24 21:23   ` J. Bruce Fields
  0 siblings, 0 replies; 4+ messages in thread
From: J. Bruce Fields @ 2020-01-24 21:23 UTC (permalink / raw)
  To: Frank Sorenson
  Cc: Roberto Bergantinos Corpas, chuck.lever, trond.myklebust, linux-nfs

On Fri, Jan 24, 2020 at 10:53:19AM -0600, Frank Sorenson wrote:
> On 1/24/20 4:11 AM, Roberto Bergantinos Corpas wrote:
> > When upcalling gssproxy, cache_head.expiry_time is set as a
> > timeval, not seconds since boot. As such, RPC cache expiry
> > logic will not clean expired objects created under
> > auth.rpcsec.context cache.

Looks like expiration times have worked this way since 2010's
c5b29f885afe "sunrpc: use seconds since boot in expiry cache".
gss_proxy_save_rsc was added in 2012 with 030d794bf498 "SUNRPC: Use
gssproxy upcall for server RPCGSS authentication", so it's the gssproxy
code that introduced the bug.  That's a while for this to lurk, but it
sounds like it required a bit of an extreme case to make it obvious.

Applying with a stable cc, Frank's Tested-by and a note on the above.
Thanks, everyone!

--b.

> > 
> > This has proven to cause kernel memory leaks on field.
> > 
> > Signed-off-by: Roberto Bergantinos Corpas <rbergant@redhat.com>
> > ---
> >  net/sunrpc/auth_gss/svcauth_gss.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
> > index 8be2f209982b..725cf5b5ae40 100644
> > --- a/net/sunrpc/auth_gss/svcauth_gss.c
> > +++ b/net/sunrpc/auth_gss/svcauth_gss.c
> > @@ -1211,6 +1211,7 @@ static int gss_proxy_save_rsc(struct cache_detail *cd,
> >  		dprintk("RPC:       No creds found!\n");
> >  		goto out;
> >  	} else {
> > +		struct timespec boot;
> >  
> >  		/* steal creds */
> >  		rsci.cred = ud->creds;
> > @@ -1231,6 +1232,9 @@ static int gss_proxy_save_rsc(struct cache_detail *cd,
> >  						&expiry, GFP_KERNEL);
> >  		if (status)
> >  			goto out;
> > +
> > +		getboottime(&boot);
> > +		expiry -= boot.tv_sec;
> >  	}
> >  
> >  	rsci.h.expiry_time = expiry;
> > 
> 
> The accumulating  become apparent when the client uses kerberos tickets
> with very short (10 seconds or fewer) lifetimes and renewable lifetimes:
> 
> mount server:/exports /mnt/tmp -overs=4,sec=krb5p
> life="2s"
> rlife="3s"
> while true ; do
> 	while true ; do
> 		kinit -l $life -R >/dev/null 2>&1 && break
> 		echo 'PASSWORD' | kinit -l $life -r $rlife \
> 			>/dev/null 2>&1 && break
> 	done
> 	timeout -k 1 2 touch /mnt/tmp/foo
> 	echo -n .
> done
> 
> Due to the entry expiration occurring 50 years in the future, the
> customer had accumulated in excess of 400,000 entries in the cache over
> about a month with just 6 nfs clients.  The entries, with all the
> accompanying structs which had been allocated consumed over 2 GiB from
> various slab caches.
> 
> A flush of the cache cleans everything out, however they will again
> accumulate afterward.
> 
> This patch fixes the expiration issue.
> 
> Tested-By: Frank Sorenson <sorenson@redhat.com>
> 
> 
> Frank
> --
> Frank Sorenson
> sorenson@redhat.com
> Principal Software Maintenance Engineer
> Global Support Services - filesystems
> Red Hat
> 
> 
> 
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] sunrpc: expiry_time should be seconds not timeval
  2020-01-24 10:11 Roberto Bergantinos Corpas
@ 2020-01-24 16:53 ` Frank Sorenson
  2020-01-24 21:23   ` J. Bruce Fields
  0 siblings, 1 reply; 4+ messages in thread
From: Frank Sorenson @ 2020-01-24 16:53 UTC (permalink / raw)
  To: Roberto Bergantinos Corpas, bfields
  Cc: chuck.lever, trond.myklebust, linux-nfs

On 1/24/20 4:11 AM, Roberto Bergantinos Corpas wrote:
> When upcalling gssproxy, cache_head.expiry_time is set as a
> timeval, not seconds since boot. As such, RPC cache expiry
> logic will not clean expired objects created under
> auth.rpcsec.context cache.
> 
> This has proven to cause kernel memory leaks on field.
> 
> Signed-off-by: Roberto Bergantinos Corpas <rbergant@redhat.com>
> ---
>  net/sunrpc/auth_gss/svcauth_gss.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
> index 8be2f209982b..725cf5b5ae40 100644
> --- a/net/sunrpc/auth_gss/svcauth_gss.c
> +++ b/net/sunrpc/auth_gss/svcauth_gss.c
> @@ -1211,6 +1211,7 @@ static int gss_proxy_save_rsc(struct cache_detail *cd,
>  		dprintk("RPC:       No creds found!\n");
>  		goto out;
>  	} else {
> +		struct timespec boot;
>  
>  		/* steal creds */
>  		rsci.cred = ud->creds;
> @@ -1231,6 +1232,9 @@ static int gss_proxy_save_rsc(struct cache_detail *cd,
>  						&expiry, GFP_KERNEL);
>  		if (status)
>  			goto out;
> +
> +		getboottime(&boot);
> +		expiry -= boot.tv_sec;
>  	}
>  
>  	rsci.h.expiry_time = expiry;
> 

The accumulating  become apparent when the client uses kerberos tickets
with very short (10 seconds or fewer) lifetimes and renewable lifetimes:

mount server:/exports /mnt/tmp -overs=4,sec=krb5p
life="2s"
rlife="3s"
while true ; do
	while true ; do
		kinit -l $life -R >/dev/null 2>&1 && break
		echo 'PASSWORD' | kinit -l $life -r $rlife \
			>/dev/null 2>&1 && break
	done
	timeout -k 1 2 touch /mnt/tmp/foo
	echo -n .
done

Due to the entry expiration occurring 50 years in the future, the
customer had accumulated in excess of 400,000 entries in the cache over
about a month with just 6 nfs clients.  The entries, with all the
accompanying structs which had been allocated consumed over 2 GiB from
various slab caches.

A flush of the cache cleans everything out, however they will again
accumulate afterward.

This patch fixes the expiration issue.

Tested-By: Frank Sorenson <sorenson@redhat.com>


Frank
--
Frank Sorenson
sorenson@redhat.com
Principal Software Maintenance Engineer
Global Support Services - filesystems
Red Hat









^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] sunrpc: expiry_time should be seconds not timeval
@ 2020-01-24 10:11 Roberto Bergantinos Corpas
  2020-01-24 16:53 ` Frank Sorenson
  0 siblings, 1 reply; 4+ messages in thread
From: Roberto Bergantinos Corpas @ 2020-01-24 10:11 UTC (permalink / raw)
  To: bfields; +Cc: chuck.lever, trond.myklebust, linux-nfs

When upcalling gssproxy, cache_head.expiry_time is set as a
timeval, not seconds since boot. As such, RPC cache expiry
logic will not clean expired objects created under
auth.rpcsec.context cache.

This has proven to cause kernel memory leaks on field.

Signed-off-by: Roberto Bergantinos Corpas <rbergant@redhat.com>
---
 net/sunrpc/auth_gss/svcauth_gss.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 8be2f209982b..725cf5b5ae40 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -1211,6 +1211,7 @@ static int gss_proxy_save_rsc(struct cache_detail *cd,
 		dprintk("RPC:       No creds found!\n");
 		goto out;
 	} else {
+		struct timespec boot;
 
 		/* steal creds */
 		rsci.cred = ud->creds;
@@ -1231,6 +1232,9 @@ static int gss_proxy_save_rsc(struct cache_detail *cd,
 						&expiry, GFP_KERNEL);
 		if (status)
 			goto out;
+
+		getboottime(&boot);
+		expiry -= boot.tv_sec;
 	}
 
 	rsci.h.expiry_time = expiry;
-- 
2.21.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-04 10:32 [PATCH] sunrpc: expiry_time should be seconds not timeval Roberto Bergantinos Corpas
  -- strict thread matches above, loose matches on Subject: below --
2020-01-24 10:11 Roberto Bergantinos Corpas
2020-01-24 16:53 ` Frank Sorenson
2020-01-24 21:23   ` J. Bruce Fields

Linux-NFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nfs/0 linux-nfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nfs linux-nfs/ https://lore.kernel.org/linux-nfs \
		linux-nfs@vger.kernel.org
	public-inbox-index linux-nfs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-nfs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git