From patchwork Wed Oct 31 20:23:22 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: NeilBrown X-Patchwork-Id: 1681431 Return-Path: X-Original-To: patchwork-linux-nfs@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id C04344005F for ; Wed, 31 Oct 2012 20:23:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755989Ab2JaUXA (ORCPT ); Wed, 31 Oct 2012 16:23:00 -0400 Received: from cantor2.suse.de ([195.135.220.15]:39819 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754524Ab2JaUW7 (ORCPT ); Wed, 31 Oct 2012 16:22:59 -0400 Received: from relay1.suse.de (unknown [195.135.220.254]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2.suse.de (Postfix) with ESMTP id 8E218A2C8B; Wed, 31 Oct 2012 21:22:58 +0100 (CET) Date: Thu, 1 Nov 2012 07:23:22 +1100 From: NeilBrown To: Chuck Lever Cc: Trond Myklebust , NFS Subject: Re: [PATCH] NFS: fix bug in legacy DNS resolver. Message-ID: <20121101072322.47bcb34a@notabene.brown> In-Reply-To: <06657F3E-5812-4680-929E-DE0FAED5601E@oracle.com> References: <20121031121601.5de16a5b@notabene.brown> <06657F3E-5812-4680-929E-DE0FAED5601E@oracle.com> X-Mailer: Claws Mail 3.7.10 (GTK+ 2.24.7; x86_64-suse-linux-gnu) Mime-Version: 1.0 Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org On Wed, 31 Oct 2012 11:09:27 -0400 Chuck Lever wrote: > Thanks, Neil! > > On Oct 30, 2012, at 9:16 PM, NeilBrown wrote: > > > > > The DNS resolver's use of the sunrpc cache involves a 'ttl' number > > (relative) rather that a timeout (absolute). This confused me when > > I wrote > > commit c5b29f885afe890f953f7f23424045cdad31d3e4 > > "sunrpc: use seconds since boot in expiry cache" > > > > and I managed to break it. The effect is that any TTL is interpreted > > as 0, and nothing useful gets into the cache. > > > > This patch removes the use of get_expiry() - which really expects an > > expiry time - and uses get_uint() instead, treating the int correctly > > as a ttl. > > Nit: The description says "get_uint()" but the patch uses "get_int()." Should we change the patch? > Yes, change the patch. I remember looking at that issue and deciding that get_uint() was the right thing to use as 'ttl' is already unsigned. And I wrote the description based on the memory. But it seems I didn't change the code. Weird. > > This fixes a regression that has been present since 2.6.37, causing > > certain NFS accesses in certain environments to incorrectly fail. > > In particular, when the legacy DNS resolver is in use, this regression prevents the NFS client from following any NFSv4 referral which contains a DNS hostname. Thanks. Updated patch below. NeilBrown From e0b382c5b880017853f6a4cc0eda472cf6fef0e4 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 31 Oct 2012 12:10:48 +1100 Subject: [PATCH] NFS: fix bug in legacy DNS resolver. The DNS resolver's use of the sunrpc cache involves a 'ttl' number (relative) rather that a timeout (absolute). This confused me when I wrote commit c5b29f885afe890f953f7f23424045cdad31d3e4 "sunrpc: use seconds since boot in expiry cache" and I managed to break it. The effect is that any TTL is interpreted as 0, and nothing useful gets into the cache. This patch removes the use of get_expiry() - which really expects an expiry time - and uses get_uint() instead, treating the int correctly as a ttl. This fixes a regression that has been present since 2.6.37, causing certain NFS accesses in certain environments to incorrectly fail. In particular, when the legacy DNS resolver is in use, this regression prevents the NFS client from following any NFSv4 referral which contains a DNS hostname. Reported-by: Chuck Lever Tested-by: Chuck Lever Cc: stable@vger.kernel.org Signed-off-by: NeilBrown diff --git a/fs/nfs/dns_resolve.c b/fs/nfs/dns_resolve.c index 31c26c4..ca4b11e 100644 --- a/fs/nfs/dns_resolve.c +++ b/fs/nfs/dns_resolve.c @@ -217,7 +217,7 @@ static int nfs_dns_parse(struct cache_detail *cd, char *buf, int buflen) { char buf1[NFS_DNS_HOSTNAME_MAXLEN+1]; struct nfs_dns_ent key, *item; - unsigned long ttl; + unsigned int ttl; ssize_t len; int ret = -EINVAL; @@ -240,7 +240,8 @@ static int nfs_dns_parse(struct cache_detail *cd, char *buf, int buflen) key.namelen = len; memset(&key.h, 0, sizeof(key.h)); - ttl = get_expiry(&buf); + if (get_uint(&buf, &ttl) < 0) + goto out; if (ttl == 0) goto out; key.h.expiry_time = ttl + seconds_since_boot();