Re: git: 29af6d2e2ec9 - main - msdosfs: replace '/' in direntries with '?'

On Fri, Jul 18, 2025 at 10:06:56AM -0600, Alan Somers wrote:
> Should we move this logic up into kern_getdirentries?  msdosfs is not the
> only file system vulnerable to this problem.
It is relatively hard to do in kern_getdirentries(), and perhaps would
cause a severe performance hit for filesystems that do not need it.

The issue is that uio might be for UIO_USERSPACE (and less likely UIO_NOCOPY).
So we must allocate the transient buffer, then call VOP_READDIR() for that
buffer, then do the validation, and then copyout to the final uio.

Another thing, there are more VOP_READDIR() uses than only kern_getdirents().
Worst part, we do trust UFS and ZFS which are the most perf-sensitive.

I did looked at generic checker, might be guided by some MNTK_-level flag,
but decided to just patch msdosfs.

> 
> On Thu, Jul 17, 2025 at 3:54 PM Konstantin Belousov <kib@freebsd.org> wrote:
> 
> > The branch main has been updated by kib:
> >
> > URL:
> > https://cgit.FreeBSD.org/src/commit/?id=29af6d2e2ec9fe8df7cf1e1a0bf3597028831b18
> >
> > commit 29af6d2e2ec9fe8df7cf1e1a0bf3597028831b18
> > Author:     Konstantin Belousov <kib@FreeBSD.org>
> > AuthorDate: 2025-07-17 01:12:05 +0000
> > Commit:     Konstantin Belousov <kib@FreeBSD.org>
> > CommitDate: 2025-07-17 21:53:54 +0000
> >
> >     msdosfs: replace '/' in direntries with '?'
> >
> >     PR:     288266
> >     Reported by:    Robert Morris <rtm@lcs.mit.edu>
> >     Reviewed by:    markj
> >     Sponsored by:   The FreeBSD Foundation
> >     MFC after:      1 week
> >     Differential revision:  https://reviews.freebsd.org/D51365
> > ---
> >  sys/fs/msdosfs/msdosfs_conv.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/sys/fs/msdosfs/msdosfs_conv.c b/sys/fs/msdosfs/msdosfs_conv.c
> > index da4848169173..208b64930e61 100644
> > --- a/sys/fs/msdosfs/msdosfs_conv.c
> > +++ b/sys/fs/msdosfs/msdosfs_conv.c
> > @@ -797,19 +797,24 @@ mbsadjpos(const char **instr, size_t inlen, size_t
> > outlen, int weight, int flag,
> >  static u_char *
> >  dos2unixchr(u_char *outbuf, const u_char **instr, size_t *ilen, int
> > lower, struct msdosfsmount *pmp)
> >  {
> > -       u_char c, *outp;
> > -       size_t len, olen;
> > +       u_char c, *outp, *outp1;
> > +       size_t i, len, olen;
> >
> >         outp = outbuf;
> >         if (pmp->pm_flags & MSDOSFSMNT_KICONV && msdosfs_iconv) {
> >                 olen = len = 4;
> >
> > +               outp1 = outp;
> >                 if (lower & (LCASE_BASE | LCASE_EXT))
> >                         msdosfs_iconv->convchr_case(pmp->pm_d2u, (const
> > char **)instr,
> >                                                   ilen, (char **)&outp,
> > &olen, KICONV_LOWER);
> >                 else
> >                         msdosfs_iconv->convchr(pmp->pm_d2u, (const char
> > **)instr,
> >                                              ilen, (char **)&outp, &olen);
> > +               for (i = 0; i < outp - outp1; i++) {
> > +                       if (outp1[i] == '/')
> > +                               outp1[i] = '?';
> > +               }
> >                 len -= olen;
> >
> >                 /*
> > @@ -826,6 +831,8 @@ dos2unixchr(u_char *outbuf, const u_char **instr,
> > size_t *ilen, int lower, struc
> >                 c = dos2unix[c];
> >                 if (lower & (LCASE_BASE | LCASE_EXT))
> >                         c = u2l[c];
> > +               if (c == '/')
> > +                       c = '?';
> >                 *outp++ = c;
> >                 outbuf[1] = '\0';
> >         }
> >