Re: vfs references to strncpy and MFSNAMELEN
I did not get any reply except yours.- Indeed patching is very easy that's why I didn't bother to post one and let most advanced users to patch it.- Testing for regressions can be hard.- I've found, in 5.4 version, upto 1400 references to strncpy but most of them should cause no problem (nor overflow possible if they function which calls it has the correct 'bounds'). As you correctly said I've missed that reference that expect a nul-terminating string, however, if the change strncpy/strlcpy is done there should be no problem to produce a nul-terminate string for the function you mention. Best regards -Original Message- From: owner-t...@openbsd.org [mailto:owner-t...@openbsd.org] On Behalf Of patrick keshishian Sent: viernes, 02 de mayo de 2014 00:17 To: tech@openbsd.org; Héctor Luis Gimbatti Subject: Re: vfs references to strncpy and MFSNAMELEN On 4/29/14, Héctor Luis Gimbatti wrote: > The constant MFSNAMELEN as defined in: > > lib/libc/sys/getfsstat.2:#define MFSNAMELEN 16 > lib/libc/sys/statfs.2:#define MFSNAMELEN 16 > sys/sys/mount.h: #define MFSNAMELEN 16 > > defines the fs type name and, according to comments, it includes nul > terminating character. > > The following code makes uses of strncpy and involves MFSNAMELEN > > ./sys/kern/vfs_subr.c:225: strncpy(mp->mnt_stat.f_fstypename, > vfsp->vfc_name, MFSNAMELEN); > ./sys/kern/vfs_subr.c:2272: strncpy(sbp->f_fstypename, > mp->mnt_vfc->vfc_name, MFSNAMELEN); > ./sys/kern/vfs_syscalls.c:243: strncpy(mp->mnt_stat.f_fstypename, > vfsp->vfc_name, MFSNAMELEN); > ./sys/miscfs/procfs/procfs_vfsops.c:190:strncpy(sbp->f_fstypename, > mp->mnt_vfc->vfc_name, MFSNAMELEN); > ./sys/msdosfs/msdosfs_vfsops.c:669: strncpy(sbp->f_fstypename, > mp->mnt_vfc->vfc_name, MFSNAMELEN); > ./sys/nfs/nfs_vfsops.c:646: strncpy(&mp->mnt_stat.f_fstypename[0], > mp->mnt_vfc->vfc_name, MFSNAMELEN); > ./sys/ntfs/ntfs_vfsops.c:628: strncpy(sbp->f_fstypename, > mp->mnt_vfc->vfc_name, MFSNAMELEN); > ./sys/ufs/ext2fs/ext2fs_vfsops.c:704: strncpy(sbp->f_fstypename, > mp->mnt_vfc->vfc_name, MFSNAMELEN); > ./sys/ufs/mfs/mfs_vfsops.c:222: strncpy(&sbp->f_fstypename[0], > mp->mnt_vfc->vfc_name, MFSNAMELEN); > > > Can be those replace safely by strlcpy without any modification to > MFSNAMELEN constant? I thought I had seen someone reply to this on tech@ but I can't find any evidence of that. So... *ping*? Most places use strn{cmp,cpy} except for compat/linux/linux_misc.c so it "seems" OK. However, OP's grep missed the following usage which (I imagine) expects a nul-terminated c-string: sys/kern/vfs_subr.c in vfs_mount_print() (*pr)(" fstype \"%s\" mnton \"%s\" mntfrom \"%s\" mntspec \"%s\"\n", mp->mnt_stat.f_fstypename, mp->mnt_stat.f_mntonname, mp->mnt_stat.f_mntfromname, mp->mnt_stat.f_mntfromspec); Called from ddb/db_command.c: db_show_all_{mount,vnode}s() Producing a patch as OP suggested is simple enough, but testing for regression is a bit of an unknown to me. --patrick
Re: vfs references to strncpy and MFSNAMELEN
On 4/29/14, H??ctor Luis Gimbatti wrote: > The constant MFSNAMELEN as defined in: > > lib/libc/sys/getfsstat.2:#define MFSNAMELEN 16 > lib/libc/sys/statfs.2:#define MFSNAMELEN 16 > sys/sys/mount.h: #define MFSNAMELEN 16 > > defines the fs type name and, according to comments, it includes nul > terminating character. > > The following code makes uses of strncpy and involves MFSNAMELEN > > ./sys/kern/vfs_subr.c:225: strncpy(mp->mnt_stat.f_fstypename, > vfsp->vfc_name, MFSNAMELEN); > ./sys/kern/vfs_subr.c:2272: strncpy(sbp->f_fstypename, > mp->mnt_vfc->vfc_name, MFSNAMELEN); > ./sys/kern/vfs_syscalls.c:243: strncpy(mp->mnt_stat.f_fstypename, > vfsp->vfc_name, MFSNAMELEN); > ./sys/miscfs/procfs/procfs_vfsops.c:190:strncpy(sbp->f_fstypename, > mp->mnt_vfc->vfc_name, MFSNAMELEN); > ./sys/msdosfs/msdosfs_vfsops.c:669: strncpy(sbp->f_fstypename, > mp->mnt_vfc->vfc_name, MFSNAMELEN); > ./sys/nfs/nfs_vfsops.c:646: strncpy(&mp->mnt_stat.f_fstypename[0], > mp->mnt_vfc->vfc_name, MFSNAMELEN); > ./sys/ntfs/ntfs_vfsops.c:628: strncpy(sbp->f_fstypename, > mp->mnt_vfc->vfc_name, MFSNAMELEN); > ./sys/ufs/ext2fs/ext2fs_vfsops.c:704: strncpy(sbp->f_fstypename, > mp->mnt_vfc->vfc_name, MFSNAMELEN); > ./sys/ufs/mfs/mfs_vfsops.c:222: strncpy(&sbp->f_fstypename[0], > mp->mnt_vfc->vfc_name, MFSNAMELEN); > > > Can be those replace safely by strlcpy without any modification to > MFSNAMELEN constant? I thought I had seen someone reply to this on tech@ but I can't find any evidence of that. So... *ping*? Most places use strn{cmp,cpy} except for compat/linux/linux_misc.c so it "seems" OK. However, OP's grep missed the following usage which (I imagine) expects a nul-terminated c-string: sys/kern/vfs_subr.c in vfs_mount_print() (*pr)(" fstype \"%s\" mnton \"%s\" mntfrom \"%s\" mntspec \"%s\"\n", mp->mnt_stat.f_fstypename, mp->mnt_stat.f_mntonname, mp->mnt_stat.f_mntfromname, mp->mnt_stat.f_mntfromspec); Called from ddb/db_command.c: db_show_all_{mount,vnode}s() Producing a patch as OP suggested is simple enough, but testing for regression is a bit of an unknown to me. --patrick
vfs references to strncpy and MFSNAMELEN
The constant MFSNAMELEN as defined in: lib/libc/sys/getfsstat.2:#define MFSNAMELEN 16 lib/libc/sys/statfs.2:#define MFSNAMELEN 16 sys/sys/mount.h: #define MFSNAMELEN 16 defines the fs type name and, according to comments, it includes nul terminating character. The following code makes uses of strncpy and involves MFSNAMELEN ./sys/kern/vfs_subr.c:225: strncpy(mp->mnt_stat.f_fstypename, vfsp->vfc_name, MFSNAMELEN); ./sys/kern/vfs_subr.c:2272: strncpy(sbp->f_fstypename, mp->mnt_vfc->vfc_name, MFSNAMELEN); ./sys/kern/vfs_syscalls.c:243: strncpy(mp->mnt_stat.f_fstypename, vfsp->vfc_name, MFSNAMELEN); ./sys/miscfs/procfs/procfs_vfsops.c:190:strncpy(sbp->f_fstypename, mp->mnt_vfc->vfc_name, MFSNAMELEN); ./sys/msdosfs/msdosfs_vfsops.c:669: strncpy(sbp->f_fstypename, mp->mnt_vfc->vfc_name, MFSNAMELEN); ./sys/nfs/nfs_vfsops.c:646: strncpy(&mp->mnt_stat.f_fstypename[0], mp->mnt_vfc->vfc_name, MFSNAMELEN); ./sys/ntfs/ntfs_vfsops.c:628: strncpy(sbp->f_fstypename, mp->mnt_vfc->vfc_name, MFSNAMELEN); ./sys/ufs/ext2fs/ext2fs_vfsops.c:704: strncpy(sbp->f_fstypename, mp->mnt_vfc->vfc_name, MFSNAMELEN); ./sys/ufs/mfs/mfs_vfsops.c:222: strncpy(&sbp->f_fstypename[0], mp->mnt_vfc->vfc_name, MFSNAMELEN); Can be those replace safely by strlcpy without any modification to MFSNAMELEN constant?