Re: Valgrind warnings for `automount`

Donald Buczek <buczek@xxxxxxxxxxxxx> · Mon, 12 Jun 2017 16:20:45 +0200

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@xxxxxxxxxxxxx
Tel: +49 30 8413 1433

--
To unsubscribe from this list: send the line "unsubscribe autofs" in