From: Steve Dickson <steved@redhat.com>
To: "Bogdan-Cristian Tătăroiu" <b.tataroiu@gmail.com>
Cc: linux-nfs@vger.kernel.org
Subject: Re: [PATCH] Add guards around [nfsidmap] usages of [sysconf].
Date: Sat, 30 Nov 2024 09:20:08 -0500 [thread overview]
Message-ID: <71ebdfc0-b7c9-44a5-916e-b73911a8fdff@redhat.com> (raw)
In-Reply-To: <CAC=XK8Nk_odMwvwXrj9suXCayni-pVnzTga0aV9tn-B-3=ia9g@mail.gmail.com>
On 11/30/24 7:32 AM, Bogdan-Cristian Tătăroiu wrote:
> Hey Steve,
>
> My apologies for dropping this.
>
> I'm a bit confused since the man page for sysconf [1] seems to say it
> returns -1 and sets errno to EINVAL (and indeed the patch I originally
> submitted shows the intended behaviour on my musl system).
>
> The snippet in the newly defined [get_pwnam_buflen] is pretty much
> just what's in the [getpwnam] man page [2] example.
>
> Maybe the thing you're pointing to is that I have [*size_t* buflen =
> sysconf(_SC_GETPW_R_SIZE_MAX)].
> I guess that should technically be [long buflen] and the result could
> be converted to [size_t] upon return, it's just that the implicit
> conversions make it such that the current code behaves as expected.
>
> So something like
>
> long buflen = sysconf(_SC_GETPW_R_SIZE_MAX);
> if (buflen == -1)
> buflen = 16384;
> return (size_t)buflen;
>
> Does that make more sense?
Yes... it does... would mind posting another patch?
I don't have access to a musl system so I can not
test the change in that world.
tia
steved.
>
> [1] https://www.man7.org/linux/man-pages/man3/sysconf.3.html
> [2] https://www.man7.org/linux/man-pages/man3/getpwnam.3.html
>
> Best,
> Bogdan
>
> On Sat, Nov 9, 2024 at 6:25 PM Steve Dickson <steved@redhat.com> wrote:
>>
>>
>>
>> On 6/17/24 2:51 PM, Steve Dickson wrote:
>>>
>>>
>>> On 5/30/24 2:17 AM, Bogdan-Cristian Tătăroiu wrote:
>>>> sysconf(_SC_GETPW_R_SIZE_MAX) and sysconf(_SC_GETGR_R_SIZE_MAX)
>>>> return -1 on musl, which causes either segmentation faults or ENOMEM
>>>> errors.
>>> Actually sysconf() returns EINVAL not -1 since the return
>>> value is a size_t (unsigned long). So I needed to change
>>>
>>> size_t buflen = sysconf(_SC_GETPW_R_SIZE_MAX);
>>> if (buflen == EINVAL) <<< this from -1 to EINVAL
>>> buflen = 16384;
>>> return buflen;
>>>
>>> Good with that? Will this work with musl?
>> Just found this on my todo list... Still
>> interest in have these patch committed?
>>
>> steved.
>>
>>>
>>> steved.
>>>
>>>>
>>>> Replace all usages of sysconf with dedicated methods that guard against
>>>> a result of -1.
>>>>
>>>> Signed-off-by: Bogdan-Cristian Tătăroiu <b.tataroiu@gmail.com>
>>>> ---
>>>> support/nfsidmap/gums.c | 4 ++--
>>>> support/nfsidmap/libnfsidmap.c | 4 ++--
>>>> support/nfsidmap/nfsidmap_common.c | 16 ++++++++++++++++
>>>> support/nfsidmap/nfsidmap_private.h | 2 ++
>>>> support/nfsidmap/nss.c | 8 ++++----
>>>> support/nfsidmap/regex.c | 9 +++++----
>>>> support/nfsidmap/static.c | 5 +++--
>>>> 7 files changed, 34 insertions(+), 14 deletions(-)
>>>>
>>>> diff --git a/support/nfsidmap/gums.c b/support/nfsidmap/gums.c
>>>> index 1d6eb318..e94a4c50 100644
>>>> --- a/support/nfsidmap/gums.c
>>>> +++ b/support/nfsidmap/gums.c
>>>> @@ -475,7 +475,7 @@ static int translate_to_uid(char *local_uid, uid_t
>>>> *uid, uid_t *gid)
>>>> int ret = -1;
>>>> struct passwd *pw = NULL;
>>>> struct pwbuf *buf = NULL;
>>>> - size_t buflen = sysconf(_SC_GETPW_R_SIZE_MAX);
>>>> + size_t buflen = get_pwnam_buflen();
>>>> buf = malloc(sizeof(*buf) + buflen);
>>>> if (buf == NULL)
>>>> @@ -501,7 +501,7 @@ static int translate_to_gid(char *local_gid, uid_t
>>>> *gid)
>>>> struct group *gr = NULL;
>>>> struct group grbuf;
>>>> char *buf = NULL;
>>>> - size_t buflen = sysconf(_SC_GETGR_R_SIZE_MAX);
>>>> + size_t buflen = get_grnam_buflen();
>>>> int ret = -1;
>>>> do {
>>>> diff --git a/support/nfsidmap/libnfsidmap.c b/support/nfsidmap/
>>>> libnfsidmap.c
>>>> index f8c36480..e1475879 100644
>>>> --- a/support/nfsidmap/libnfsidmap.c
>>>> +++ b/support/nfsidmap/libnfsidmap.c
>>>> @@ -457,7 +457,7 @@ int nfs4_init_name_mapping(char *conffile)
>>>> nobody_user = conf_get_str("Mapping", "Nobody-User");
>>>> if (nobody_user) {
>>>> - size_t buflen = sysconf(_SC_GETPW_R_SIZE_MAX);
>>>> + size_t buflen = get_pwnam_buflen();
>>>> struct passwd *buf;
>>>> struct passwd *pw = NULL;
>>>> int err;
>>>> @@ -478,7 +478,7 @@ int nfs4_init_name_mapping(char *conffile)
>>>> nobody_group = conf_get_str("Mapping", "Nobody-Group");
>>>> if (nobody_group) {
>>>> - size_t buflen = sysconf(_SC_GETGR_R_SIZE_MAX);
>>>> + size_t buflen = get_grnam_buflen();
>>>> struct group *buf;
>>>> struct group *gr = NULL;
>>>> int err;
>>>> diff --git a/support/nfsidmap/nfsidmap_common.c b/support/nfsidmap/
>>>> nfsidmap_common.c
>>>> index 4d2cb14f..310c68f0 100644
>>>> --- a/support/nfsidmap/nfsidmap_common.c
>>>> +++ b/support/nfsidmap/nfsidmap_common.c
>>>> @@ -116,3 +116,19 @@ int get_reformat_group(void)
>>>> return reformat_group;
>>>> }
>>>> +
>>>> +size_t get_pwnam_buflen(void)
>>>> +{
>>>> + size_t buflen = sysconf(_SC_GETPW_R_SIZE_MAX);
>>>> + if (buflen == -1)
>>>> + buflen = 16384;
>>>> + return buflen;
>>>> +}
>>>> +
>>>> +size_t get_grnam_buflen(void)
>>>> +{
>>>> + size_t buflen = sysconf(_SC_GETGR_R_SIZE_MAX);
>>>> + if (buflen == -1)
>>>> + buflen = 16384;
>>>> + return buflen;
>>>> +}
>>>> diff --git a/support/nfsidmap/nfsidmap_private.h b/support/nfsidmap/
>>>> nfsidmap_private.h
>>>> index a5cb6dda..234ca9d4 100644
>>>> --- a/support/nfsidmap/nfsidmap_private.h
>>>> +++ b/support/nfsidmap/nfsidmap_private.h
>>>> @@ -40,6 +40,8 @@ struct conf_list *get_local_realms(void);
>>>> void free_local_realms(void);
>>>> int get_nostrip(void);
>>>> int get_reformat_group(void);
>>>> +size_t get_pwnam_buflen(void);
>>>> +size_t get_grnam_buflen(void);
>>>> typedef enum {
>>>> IDTYPE_USER = 1,
>>>> diff --git a/support/nfsidmap/nss.c b/support/nfsidmap/nss.c
>>>> index 0f43076e..3fc045dc 100644
>>>> --- a/support/nfsidmap/nss.c
>>>> +++ b/support/nfsidmap/nss.c
>>>> @@ -91,7 +91,7 @@ static int nss_uid_to_name(uid_t uid, char *domain,
>>>> char *name, size_t len)
>>>> struct passwd *pw = NULL;
>>>> struct passwd pwbuf;
>>>> char *buf;
>>>> - size_t buflen = sysconf(_SC_GETPW_R_SIZE_MAX);
>>>> + size_t buflen = get_pwnam_buflen();
>>>> int err = -ENOMEM;
>>>> buf = malloc(buflen);
>>>> @@ -119,7 +119,7 @@ static int nss_gid_to_name(gid_t gid, char
>>>> *domain, char *name, size_t len)
>>>> struct group *gr = NULL;
>>>> struct group grbuf;
>>>> char *buf;
>>>> - size_t buflen = sysconf(_SC_GETGR_R_SIZE_MAX);
>>>> + size_t buflen = get_grnam_buflen();
>>>> int err;
>>>> if (domain == NULL)
>>>> @@ -192,7 +192,7 @@ static struct passwd *nss_getpwnam(const char
>>>> *name, const char *domain,
>>>> {
>>>> struct passwd *pw;
>>>> struct pwbuf *buf;
>>>> - size_t buflen = sysconf(_SC_GETPW_R_SIZE_MAX);
>>>> + size_t buflen = get_pwnam_buflen();
>>>> char *localname;
>>>> int err = ENOMEM;
>>>> @@ -301,7 +301,7 @@ static int _nss_name_to_gid(char *name, gid_t
>>>> *gid, int dostrip)
>>>> struct group *gr = NULL;
>>>> struct group grbuf;
>>>> char *buf, *domain;
>>>> - size_t buflen = sysconf(_SC_GETGR_R_SIZE_MAX);
>>>> + size_t buflen = get_grnam_buflen();
>>>> int err = -EINVAL;
>>>> char *localname = NULL;
>>>> char *ref_name = NULL;
>>>> diff --git a/support/nfsidmap/regex.c b/support/nfsidmap/regex.c
>>>> index 8424179f..ea094b95 100644
>>>> --- a/support/nfsidmap/regex.c
>>>> +++ b/support/nfsidmap/regex.c
>>>> @@ -46,6 +46,7 @@
>>>> #include "nfsidmap.h"
>>>> #include "nfsidmap_plugin.h"
>>>> +#include "nfsidmap_private.h"
>>>> #define CONFIG_GET_STRING nfsidmap_config_get
>>>> extern const char *nfsidmap_config_get(const char *, const char *);
>>>> @@ -95,7 +96,7 @@ static struct passwd *regex_getpwnam(const char
>>>> *name, const char *UNUSED(domain
>>>> {
>>>> struct passwd *pw;
>>>> struct pwbuf *buf;
>>>> - size_t buflen = sysconf(_SC_GETPW_R_SIZE_MAX);
>>>> + size_t buflen = get_pwnam_buflen();
>>>> char *localname;
>>>> size_t namelen;
>>>> int err;
>>>> @@ -175,7 +176,7 @@ static struct group *regex_getgrnam(const char
>>>> *name, const char *UNUSED(domain)
>>>> {
>>>> struct group *gr;
>>>> struct grbuf *buf;
>>>> - size_t buflen = sysconf(_SC_GETGR_R_SIZE_MAX);
>>>> + size_t buflen = get_grnam_buflen();
>>>> char *localgroup;
>>>> char *groupname;
>>>> size_t namelen;
>>>> @@ -366,7 +367,7 @@ static int regex_uid_to_name(uid_t uid, char
>>>> *domain, char *name, size_t len)
>>>> struct passwd *pw = NULL;
>>>> struct passwd pwbuf;
>>>> char *buf;
>>>> - size_t buflen = sysconf(_SC_GETPW_R_SIZE_MAX);
>>>> + size_t buflen = get_pwnam_buflen();
>>>> int err = -ENOMEM;
>>>> buf = malloc(buflen);
>>>> @@ -392,7 +393,7 @@ static int regex_gid_to_name(gid_t gid, char
>>>> *UNUSED(domain), char *name, size_t
>>>> struct group grbuf;
>>>> char *buf;
>>>> const char *name_prefix;
>>>> - size_t buflen = sysconf(_SC_GETGR_R_SIZE_MAX);
>>>> + size_t buflen = get_grnam_buflen();
>>>> int err;
>>>> char * groupname = NULL;
>>>> diff --git a/support/nfsidmap/static.c b/support/nfsidmap/static.c
>>>> index 8ac4a398..395cac06 100644
>>>> --- a/support/nfsidmap/static.c
>>>> +++ b/support/nfsidmap/static.c
>>>> @@ -44,6 +44,7 @@
>>>> #include "conffile.h"
>>>> #include "nfsidmap.h"
>>>> #include "nfsidmap_plugin.h"
>>>> +#include "nfsidmap_private.h"
>>>> /*
>>>> * Static Translation Methods
>>>> @@ -98,7 +99,7 @@ static struct passwd *static_getpwnam(const char *name,
>>>> {
>>>> struct passwd *pw;
>>>> struct pwbuf *buf;
>>>> - size_t buflen = sysconf(_SC_GETPW_R_SIZE_MAX);
>>>> + size_t buflen = get_pwnam_buflen();
>>>> char *localname;
>>>> int err;
>>>> @@ -149,7 +150,7 @@ static struct group *static_getgrnam(const char
>>>> *name,
>>>> {
>>>> struct group *gr;
>>>> struct grbuf *buf;
>>>> - size_t buflen = sysconf(_SC_GETGR_R_SIZE_MAX);
>>>> + size_t buflen = get_grnam_buflen();
>>>> char *localgroup;
>>>> int err;
>>
>
next prev parent reply other threads:[~2024-11-30 14:20 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-30 7:17 [PATCH] Add guards around [nfsidmap] usages of [sysconf] Bogdan-Cristian Tătăroiu
2024-06-17 19:51 ` Steve Dickson
2024-11-09 18:25 ` Steve Dickson
2024-11-30 12:32 ` Bogdan-Cristian Tătăroiu
2024-11-30 14:20 ` Steve Dickson [this message]
2024-12-01 9:21 ` Bogdan-Cristian Tătăroiu
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=71ebdfc0-b7c9-44a5-916e-b73911a8fdff@redhat.com \
--to=steved@redhat.com \
--cc=b.tataroiu@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox