From: Donald Buczek <buczek@molgen.mpg.de>
To: Paul Menzel <pmenzel@molgen.mpg.de>, autofs@vger.kernel.org
Cc: it+autofs@molgen.mpg.de
Subject: Re: Valgrind warnings for `automount`
Date: Mon, 12 Jun 2017 16:20:45 +0200 [thread overview]
Message-ID: <b253d607-7714-d5b5-31fc-f5b2533ef877@molgen.mpg.de> (raw)
In-Reply-To: <e60981ac-2b4e-0dab-1333-2bf88160bda2@molgen.mpg.de>
On 06/08/17 15:14, Paul Menzel wrote:
> Dear autofs folks,
>
>
> Valgrind 3.12.0 shows the following when running `automount`.
>
> ```
> ==63563== Thread 12:
> ==63563== Conditional jump or move depends on uninitialised value(s)
> ==63563== at 0x4C2E826: rawmemchr (vg_replace_strmem.c:1402)
> ==63563== by 0x61D3FB6: _nss_files_parse_grent (fgetgrent_r.c:37)
> ==63563== by 0xB3CA5FA: internal_getent (files-XXX.c:272)
> ==63563== by 0xB3CA9E0: _nss_files_getgrgid_r (files-grp.c:39)
> ==63563== by 0x61D393B: getgrgid_r@@GLIBC_2.2.5 (getXXbyYY_r.c:266)
> ==63563== by 0x131AC3: set_tsd_user_vars (mounts.c:1538)
> ==63563== by 0x11B652: do_mount_indirect (indirect.c:776)
> ==63563== by 0x4E3B190: start_thread (pthread_create.c:309)
> ==63563== Uninitialised value was created by a heap allocation
> ==63563== at 0x4C27A10: malloc (vg_replace_malloc.c:298)
> ==63563== by 0x4C299DC: realloc (vg_replace_malloc.c:785)
> ==63563== by 0x131A4D: set_tsd_user_vars (mounts.c:1530)
> ==63563== by 0x11B652: do_mount_indirect (indirect.c:776)
> ==63563== by 0x4E3B190: start_thread (pthread_create.c:309)
> ==63563==
> ==63563== Conditional jump or move depends on uninitialised value(s)
> ==63563== at 0x4C2A4A5: __GI_strchr (vg_replace_strmem.c:246)
> ==63563== by 0x61D3DBB: _nss_files_parse_grent (fgetgrent_r.c:37)
> ==63563== by 0xB3CA5FA: internal_getent (files-XXX.c:272)
> ==63563== by 0xB3CA9E0: _nss_files_getgrgid_r (files-grp.c:39)
> ==63563== by 0x61D393B: getgrgid_r@@GLIBC_2.2.5 (getXXbyYY_r.c:266)
> ==63563== by 0x131AC3: set_tsd_user_vars (mounts.c:1538)
> ==63563== by 0x11B652: do_mount_indirect (indirect.c:776)
> ==63563== by 0x4E3B190: start_thread (pthread_create.c:309)
> ==63563== Uninitialised value was created by a heap allocation
> ==63563== at 0x4C27A10: malloc (vg_replace_malloc.c:298)
> ==63563== by 0x4C299DC: realloc (vg_replace_malloc.c:785)
> ==63563== by 0x131A4D: set_tsd_user_vars (mounts.c:1530)
> ==63563== by 0x11B652: do_mount_indirect (indirect.c:776)
> ==63563== by 0x4E3B190: start_thread (pthread_create.c:309)
> ==63563==
> ==63563== Conditional jump or move depends on uninitialised value(s)
> ==63563== at 0x4C2A49A: __GI_strchr (vg_replace_strmem.c:246)
> ==63563== by 0x61D3DBB: _nss_files_parse_grent (fgetgrent_r.c:37)
> ==63563== by 0xB3CA5FA: internal_getent (files-XXX.c:272)
> ==63563== by 0xB3CA9E0: _nss_files_getgrgid_r (files-grp.c:39)
> ==63563== by 0x61D393B: getgrgid_r@@GLIBC_2.2.5 (getXXbyYY_r.c:266)
> ==63563== by 0x131AC3: set_tsd_user_vars (mounts.c:1538)
> ==63563== by 0x11B652: do_mount_indirect (indirect.c:776)
> ==63563== by 0x4E3B190: start_thread (pthread_create.c:309)
> ==63563== Uninitialised value was created by a heap allocation
> ==63563== at 0x4C27A10: malloc (vg_replace_malloc.c:298)
> ==63563== by 0x4C299DC: realloc (vg_replace_malloc.c:785)
> ==63563== by 0x131A4D: set_tsd_user_vars (mounts.c:1530)
> ==63563== by 0x11B652: do_mount_indirect (indirect.c:776)
> ==63563== by 0x4E3B190: start_thread (pthread_create.c:309)
> ==63563==
> ```
>
> The corresponding code looks like below.
>
> ```
> gr_tmp = NULL;
> status = ERANGE;
> #ifdef ENABLE_LIMIT_GETGRGID_SIZE
> if (!maxgrpbuf)
> maxgrpbuf = detached_thread_stack_size * 0.9;
> #endif
>
> /* If getting the group name fails go on without it. It's
> * used to set an environment variable for program maps
> * which may or may not use it so it isn't critical to
> * operation.
> */
>
> tmplen = grplen;
> while (1) {
> char *tmp = realloc(gr_tmp, tmplen + 1);
> if (!tmp) {
> error(logopt, "failed to malloc buffer for
> getgrgid_r");
> goto no_group;
> }
> gr_tmp = tmp;
> pgr = &gr;
> ppgr = &pgr;
> status = getgrgid_r(gid, pgr, gr_tmp, tmplen, ppgr);
> if (status != ERANGE)
> break;
> tmplen += grplen;
>
> /* Don't tempt glibc to alloca() larger than is (likely)
> * available on the stack if limit-getgrgid-size is
> enabled.
> */
> if (!maxgrpbuf || (tmplen < maxgrpbuf))
> continue;
>
> /* Add a message so we know this happened */
> debug(logopt, "group buffer allocation would be too
> large");
> break;
> }
> ```
>
> Unfortunately, I don’t spot the problem. Do you see it?
>
>
> Kind regards,
>
> Paul
This is a bug in glibc 2.19, which is fixed in glibc 2.20.
With glibc-2.19 plus commit
https://sourceware.org/git/?p=glibc.git;a=commit;f=nss/nss_files/files-XXX.c;h=ac60763eac3d43b7234dd21286ad3ec3f17957fc
the warning is gone. Although I expect
https://sourceware.org/git/?p=glibc.git;a=commit;f=nss/nss_files/files-XXX.c;h=d7b00f98106a0f1e3d753b135eeb97dfdf6e2e74
which is included in glibc-2.19, to be part of the solution, too.
Donald
--
Donald Buczek
buczek@molgen.mpg.de
Tel: +49 30 8413 1433
--
To unsubscribe from this list: send the line "unsubscribe autofs" in
prev parent reply other threads:[~2017-06-12 14:20 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-08 13:14 Valgrind warnings for `automount` Paul Menzel
2017-06-11 23:42 ` Ian Kent
2017-06-12 14:20 ` Donald Buczek [this message]
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=b253d607-7714-d5b5-31fc-f5b2533ef877@molgen.mpg.de \
--to=buczek@molgen.mpg.de \
--cc=autofs@vger.kernel.org \
--cc=it+autofs@molgen.mpg.de \
--cc=pmenzel@molgen.mpg.de \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).