Date: Thu, 13 Aug 2020 18:09:27 -0400 From: Jung-uk Kim <jkim@FreeBSD.org> To: Martin Simmons <martin@lispworks.com>, "Mikhail T." <mi+t@aldan.algebra.com> Cc: mmokhi@FreeBSD.org, vbox@FreeBSD.org Subject: Re: Old bug in patch-src_VBox_Additions_freebsd_vboxvfs_vboxvfs__vfsops.c Message-ID: <9fe11d97-7c73-1c0f-a318-3553e2bfb2b0@FreeBSD.org> In-Reply-To: <202006231847.05NIl6eH007223@higson.cam.lispworks.com> References: <101fc234-e5c2-e8c3-9c07-3eb922c01736@aldan.algebra.com> <202006231847.05NIl6eH007223@higson.cam.lispworks.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 20. 6. 23., Martin Simmons wrote: > Is the memcpy wrong too? It looks like fsinfo.serial is a single uint32_t > (i.e. 32 bits), but mp->mnt_stat.f_fsid is a pair of int32_t (i.e. 64 bits), > so copying this pair from fsinfo.serial is wrong. You're right. I just committed a fix (r544846). Sorry for the late response. Jung-uk Kim >>>>>> On Mon, 22 Jun 2020 14:22:05 -0400, Mikhail T said: >> >> Gentlemen! >> >> An old bug in the patch is causing compiler-warnings, and leads to >> erroneous behavior where pointers are bigger than 32-bit. >> >> Moreover, given the memcpy right after it, the bzero is simply not >> needed at all. Instead of removing the bogus ampersand, the entire line >> should be deleted. (I would've replaced the memcpy with an assignment >> too, but that's not as pressing.) >> >> Can I commit this? >> >> Index: files/patch-src_VBox_Additions_freebsd_vboxvfs_vboxvfs__vfsops.c >> =================================================================== >> --- files/patch-src_VBox_Additions_freebsd_vboxvfs_vboxvfs__vfsops.c >> (revision 539883) >> +++ files/patch-src_VBox_Additions_freebsd_vboxvfs_vboxvfs__vfsops.c >> (working copy) >> @@ -11,7 +11,7 @@ >> * >> * This file is part of VirtualBox Open Source Edition (OSE), as >> * available from http://www.virtualbox.org. This file is free >> software; >> -@@ -14,245 +9,479 @@ >> +@@ -14,245 +9,478 @@ >> * VirtualBox OSE distribution. VirtualBox OSE is distributed in the >> * hope that it will be useful, but WITHOUT ANY WARRANTY of any kind. >> */ >> @@ -466,7 +466,6 @@ >> + >> + MNT_ILOCK(mp); >> + mp->mnt_data = vboxfsmp; >> *-+ bzero(&mp->mnt_stat.f_fsid, sizeof(&mp->mnt_stat.f_fsid));* >> + /* f_fsid is int32_t but serial is uint32_t, convert */ >> + memcpy(&mp->mnt_stat.f_fsid, &fsinfo.serial, >> sizeof(mp->mnt_stat.f_fsid)); >> + mp->mnt_flag |= MNT_LOCAL;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?9fe11d97-7c73-1c0f-a318-3553e2bfb2b0>