Date: Tue, 28 Nov 2006 12:13:29 +0100 (CET) From: Oliver Fromme <olli@lurza.secnetix.de> To: bde@zeta.org.au (Bruce Evans) Cc: freebsd-fs@freebsd.org Subject: Re: file creation timestamps wrong on msdos fs? Message-ID: <200611281113.kASBDTLW095435@lurza.secnetix.de> In-Reply-To: <20061128200917.W1917@delplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Bruce Evans wrote:
> Oliver Fromme wrote:
> > There seems to be a bug in src/sys/fs/msdosfs/msdosfs_vnops.c
> > because of a subtle confusion between what msdosfs calls
> > "ctime" (creation time) and what UNIX calls "ctime" (inode
> > change time, unsupported by msdosfs). If your -current is
> > older than 2006-10-24, look for these lines:
> >
> > dos2unixtime(dep->de_MDate, dep->de_MTime, 0, &vap->va_mtime);
> > if (pmp->pm_flags & MSDOSFSMNT_LONGNAME) {
> > dos2unixtime(dep->de_ADate, 0, 0, &vap->va_atime);
> > dos2unixtime(dep->de_CDate, dep->de_CTime, dep->de_CHun, &vap->va_ctime);
> > } else {
> > vap->va_atime = vap->va_mtime;
> > vap->va_ctime = vap->va_mtime;
> > }
>
> That gives a bogus ctime in the MSDOSFSMNT_LONGNAME case.
Yes, that's what I meant. Actually the current code
contains two bugs:
-1- It initializes the ctime to a wrong value (i.e. to the
birthtime of the FAT entry, which is confusingly called
"CTime").
-2- It doesn't initialize the birthtime at all.
> The bogus
> birthtime is apparently given in both cases by using vap->va_birthtime
> uninitialized, so it is stack garbage. va_birthtime isn't referenced by
> any file system except ffs, but a few file systems initialize it to
> 0 by bzero()ing the whole *vap, and least 1 file system initializes
> it accidentally correctly using VATTR_NULL().
Well, FAT _does_ support a birthtime ("CTime") in the
MSDOSFSMNT_LONGNAME case, so it makes sense to use that
value to initialize the vap->birthtime.
In the !MSDOSFSMNT_LONGNAME case, the vap->birthtime should
be zeroed or set to -1 (either is fine with me, but I agree
that -1 for an unsupported birthtime makes sense).
> > However, the question remains what the vnode's ctime should
> > be set to. There's no such thing as an inode change time
> > in FAT's directory entries. Maybe it should simply be
> > copied from the mtime.
>
> There's nothing better. msdosfs already does this for the
> !MSDOSFSMNT_LONGNAME case. Utilities expect the ctime to be set to
> an actual time, so it should't be set to -1 like the btime^H^H^H^Hirthtime.
OK, so I propose to change the above code to this:
dos2unixtime(dep->de_MDate, dep->de_MTime, 0, &vap->va_mtime);
vap->va_ctime = vap->va_mtime;
if (pmp->pm_flags & MSDOSFSMNT_LONGNAME) {
dos2unixtime(dep->de_ADate, 0, 0, &vap->va_atime);
dos2unixtime(dep->de_CDate, dep->de_CTime, dep->de_CHun,
&vap->va_birthtime);
} else {
vap->va_atime = vap->va_mtime;
vap->va_birthtime = (time_t) -1;
}
That's for RELENG_6. In HEAD, dos2unixtime() is replaced
with fattime2timespec(), but the rest should be the same.
Rene's PR hasn't showed up in gnats yet ... Should I submit
a fresh PR with a proper patch, or is it not needed?
Best regards
Oliver
--
Oliver Fromme, secnetix GmbH & Co. KG, Marktplatz 29, 85567 Grafing
Dienstleistungen mit Schwerpunkt FreeBSD: http://www.secnetix.de/bsd
Any opinions expressed in this message may be personal to the author
and may not necessarily reflect the opinions of secnetix in any way.
"... there are two ways of constructing a software design: One way
is to make it so simple that there are _obviously_ no deficiencies and
the other way is to make it so complicated that there are no _obvious_
deficiencies." -- C.A.R. Hoare, ACM Turing Award Lecture, 1980
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200611281113.kASBDTLW095435>