From: Neil Brown <neilb@suse.de>
To: Chuck Lever <chuck.lever@oracle.com>, linux-nfs@vger.kernel.org
Subject: Possible issue with nfs-utils patch: mountd: Replace "struct hostent" with "struct addrinfo"
Date: Tue, 3 Aug 2010 17:24:09 +1000 [thread overview]
Message-ID: <20100803172409.1961c5e1@notabene> (raw)
Hi Chuck,
I've been reviewing some recent nfs-utils commits prior to possibly
including them in a suse product.
I think there is part of
commit fbc1b2c8e47ee9371c3565b266f1386575b46920
Author: Chuck Lever <chuck.lever@oracle.com>
Date: Tue Jun 22 12:43:01 2010 -0400
mountd: Replace "struct hostent" with "struct addrinfo"
which is wrong. However I'm not sure if it actually breaks something or if
the relevant code is effectively dead.
The questionable chunk is in check_netgroup
/* If hname is ip address convert to FQDN */
- if (inet_aton(hname, &addr.sin_addr) &&
- (nhp = gethostbyaddr((const char *)&(addr.sin_addr),
- sizeof(addr.sin_addr), AF_INET))) {
- hname = nhp->h_name;
+ tmp = host_pton(hname);
+ if (tmp != NULL) {
+ freeaddrinfo(tmp);
if (innetgr(netgroup, hname, NULL, NULL))
return 1;
}
The new code is effectively a no-op - it conditionally calls
if (innetgr(netgroup, hname, NULL, NULL))
return 1;
and if that succeeds, it would have already succeeded near the top of the
function so we would not get here.
The original code checked if 'hname' looked like an IP address. If it did it
looked up the canonical name and checked that for membership in the netgroup.
I'm not entirely sure when check_netgroup (or client_check) would be called
with such a string in a context where doing a lookup would make sense. I
hunted through the code and found some rmtab manipulations that could result
in this, but I'm not sure.
This code was originally added about 9 years ago in what is now commit
86ae664e66c439354cb4f959e9f289059e7760a4
which blames it on Anne Milicia <milicia@missioncriticallinux.com> without
any real explanation. I don't suppose Anne is still around ???
Did you think about this at all when you made the patch? Do you have any
thoughts about whether that section of code is still needed? I don't feel
very comfortable removing it without knowing why it is there, and I don't
really know why it is there....
Thanks,
NeilBrown
next reply other threads:[~2010-08-03 7:24 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-08-03 7:24 Neil Brown [this message]
2010-08-03 15:11 ` Possible issue with nfs-utils patch: mountd: Replace "struct hostent" with "struct addrinfo" Chuck Lever
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=20100803172409.1961c5e1@notabene \
--to=neilb@suse.de \
--cc=chuck.lever@oracle.com \
--cc=linux-nfs@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.