msdosfs: Prevent buffer overflow when expanding win95 names

[kp]

Summary

In win2unixfn() we expand Windows 95 style long names. In some cases that
requires moving the data in the nbp->nb_buf buffer backwards to make room. That
code failed to check for overflows, leading to a stack overflow in win2unixfn().

We now check for this event, and mark the entire conversion as failed in that
case. This means we present the 8 character, dos style, name instead.

PR: 204643

Test Plan

Using this image:
https://people.freebsd.org/~kp/readdir.img.bz2

mdconfig readdir.img
mount_msdosfs -o longnames -D cp1251 -L ru_RU.UTF-8 /dev/md0 /mnt
ls /mnt

[pfg]

I only started looking at it.
mbnambuf_write() doesn't exist on NetBSD (or OpenBSD), and it looks like a good place to check for the overflow.

[kp]

Break up long lines.

[pfg]

Looks good to me.

[jilles]

This is good because it looks like it will at least prevent the panic, but it may not make the files accessible since there need not be a short name. Adding a comment may be appropriate.

Regarding style, the new return value uses an uncommon convention (outside code related to Linux).

sys/fs/msdosfs/msdosfs_conv.c

1031

Returning a negative error number is Linux style. We usually return a positive error number or 0 if successful in this kind of function. (If there is both a non-trivial successful return value and an error number, the successful return value is passed by reference.)

The caller usually stores the return value in a variable called error.

1042

Doh! It detected the problem but simply continued on anyway.

[kp]

• rename 'ret' to 'error'
• Use positive error numbers

[kp]

In D5977#129496, @jilles wrote:

This is good because it looks like it will at least prevent the panic, but it may not make the files accessible since there need not be a short name. Adding a comment may be appropriate.

It's my understanding that there should always be a short filename (to keep the fs usable as a plain fat fs). I've deliberately chosen to use that one instead of truncating the name (as netbsd does) to avoid getting in the situation where two directory entries have the same (truncated) name.

[jilles]

In D5977#129534, @kristof wrote:

In D5977#129496, @jilles wrote:

This is good because it looks like it will at least prevent the panic, but it may not make the files accessible since there need not be a short name. Adding a comment may be appropriate.

It's my understanding that there should always be a short filename (to keep the fs usable as a plain fat fs). I've deliberately chosen to use that one instead of truncating the name (as netbsd does) to avoid getting in the situation where two directory entries have the same (truncated) name.

Searching more, it appears that short filenames are optional in NTFS, but not in FAT. This means that FreeBSD can still be affected by this problem but not in msdosfs.

The change looks good to me.

[kp]

Committed (with further style fixes suggested by Bruce Evans) as r298664.