svn commit: r347066 - in head: sbin/fsck_ffs sys/ufs/ufs

[Conrad Meyer]

Hi Kirk,

Coverity points out that namlen may be used uninitialized in the
following sequence (CID 1401317):

On Fri, May 3, 2019 at 2:54 PM Kirk McKusick <mckusick at freebsd.org> wrote:
>
> Author: mckusick
> Date: Fri May  3 21:54:14 2019
> New Revision: 347066
> URL: https://svnweb.freebsd.org/changeset/base/347066
>
> Log:
>   This update eliminates a kernel stack disclosure bug in UFS/FFS
>   directory entries that is caused by uninitialized directory entry
>   padding written to the disk.
> ...
> --- head/sbin/fsck_ffs/dir.c    Fri May  3 21:48:42 2019        (r347065)
> +++ head/sbin/fsck_ffs/dir.c    Fri May  3 21:54:14 2019        (r347066)
> ...
> @@ -209,15 +230,39 @@ dircheck(struct inodesc *idesc, struct direct *dp)
>         char *cp;
>         u_char type;
>         u_int8_t namlen;
> -       int spaceleft;
> +       int spaceleft, modified, unused;
>
> +       modified = 0;
>         spaceleft = DIRBLKSIZ - (idesc->id_loc % DIRBLKSIZ);
>         if (dp->d_reclen == 0 ||
>             dp->d_reclen > spaceleft ||
> -           (dp->d_reclen & 0x3) != 0)
> +           (dp->d_reclen & (DIR_ROUNDUP - 1)) != 0)
>                 goto bad;
> -       if (dp->d_ino == 0)
> -               return (1);
> +       if (dp->d_ino == 0) {

In this case, namlen may never be initialized.

> +               /*
> +                * Special case of an unused directory entry. Normally
> +                * the kernel would coalesce unused space with the previous
> +                * entry by extending its d_reclen, but there are situations
> +                * (e.g. fsck) where that doesn't occur.
> +                * If we're clearing out directory cruft (-z flag), then make
> +                * sure this entry gets fully cleared as well.
> +                */
> +               if (zflag && fswritefd >= 0) {
> +                       if (dp->d_type != 0) {
> +                               dp->d_type = 0;
> +                               modified = 1;
> +                       }
> +                       if (dp->d_namlen != 0) {
> +                               dp->d_namlen = 0;
> +                               modified = 1;
> +                       }
> +                       if (dp->d_name[0] != '\0') {
> +                               dp->d_name[0] = '\0';
> +                               modified = 1;
> +                       }
> +               }
> +               goto good;

Then we jump 'good'.

> +       }
>         size = DIRSIZ(0, dp);
>         namlen = dp->d_namlen;
>         type = dp->d_type;
> @@ -231,7 +276,37 @@ dircheck(struct inodesc *idesc, struct direct *dp)
>                         goto bad;
>         if (*cp != '\0')
>                 goto bad;
> +
> +good:
> +       if (zflag && fswritefd >= 0) {
> +               /*
> +                * Clear unused directory entry space, including the d_name
> +                * padding.
> +                */
> +               /* First figure the number of pad bytes. */
> +               unused = roundup2(namlen + 1, DIR_ROUNDUP) - (namlen + 1);

And here we access uninitialized 'namlen'.

Best,
Conrad