On Mon, 12 Jun 2017, Martijn Rijkeboer wrote:
> After upgrading to the latest snapshot there seems to be something wrong
> with the msdos filesystem driver. When I copy a binary file on a msdos (fat32)
> mounted partition the content changes e.g.:
>
> # cp refind_x64.efi bootx64.efi
> # ls -l refind_x64.efi bootx64.efi
> -rw-r--r-- 1 root wheel 230856 Jun 12 10:47 bootx64.efi
> -rw-r--r-- 1 root wheel 230856 Mar 9 11:09 refind_x64.efi
> # sha256 refind_x64.efi bootx64.efi
> SHA256 (refind_x64.efi) =
> 9c9a38f168fed270c158ff5a68d3fa5172eb15bcb41662abf69faa13d2abc418
> SHA256 (bootx64.efi) =
> 26f7350143cc897d53b0257dbf5d9f1d84eace4746cbd9f2ff95a033ee0c577f
Sigh.
Please try if the attached patch fixes the problem. It reverts a likely
culprit.
Cheers,
Stefan
diff --git sys/msdosfs/denode.h sys/msdosfs/denode.h
index efa8192a06d..cdca90500ab 100644
--- sys/msdosfs/denode.h
+++ sys/msdosfs/denode.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: denode.h,v 1.31 2017/05/29 13:48:12 sf Exp $ */
+/* $OpenBSD: denode.h,v 1.30 2016/08/30 19:47:23 sf Exp $ */
/* $NetBSD: denode.h,v 1.24 1997/10/17 11:23:39 ws Exp $ */
/*-
@@ -142,6 +142,7 @@ struct denode {
struct vnode *de_devvp; /* vnode of blk dev we live on */
uint32_t de_flag; /* flag bits */
dev_t de_dev; /* device where direntry lives */
+ daddr_t de_lastr;
uint32_t de_dirclust; /* cluster of the directory file containing
this entry */
uint32_t de_diroffset; /* offset of this entry in the directory
cluster */
uint32_t de_fndoffset; /* offset of found dir entry */
diff --git sys/msdosfs/msdosfs_vnops.c sys/msdosfs/msdosfs_vnops.c
index 61b45af6185..39c60044df5 100644
--- sys/msdosfs/msdosfs_vnops.c
+++ sys/msdosfs/msdosfs_vnops.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: msdosfs_vnops.c,v 1.114 2017/05/29 13:48:12 sf Exp $ */
+/* $OpenBSD: msdosfs_vnops.c,v 1.113 2016/08/30 19:47:23 sf Exp $ */
/* $NetBSD: msdosfs_vnops.c,v 1.63 1997/10/17 11:24:19 ws Exp $ */
/*-
@@ -77,13 +77,13 @@
static uint32_t fileidhash(uint64_t);
-int msdosfs_bmaparray(struct vnode *, daddr_t, daddr_t *, int *);
int msdosfs_kqfilter(void *);
int filt_msdosfsread(struct knote *, long);
int filt_msdosfswrite(struct knote *, long);
int filt_msdosfsvnode(struct knote *, long);
void filt_msdosfsdetach(struct knote *);
+
/*
* Some general notes:
*
@@ -509,14 +509,18 @@ int
msdosfs_read(void *v)
{
struct vop_read_args *ap = v;
+ int error = 0;
+ uint32_t diff;
+ int blsize;
+ int isadir;
+ uint32_t n;
+ long on;
+ daddr_t lbn, rablock, rablkno;
+ struct buf *bp;
struct vnode *vp = ap->a_vp;
struct denode *dep = VTODE(vp);
struct msdosfsmount *pmp = dep->de_pmp;
struct uio *uio = ap->a_uio;
- int isadir, error = 0;
- uint32_t n, diff, size, on;
- struct buf *bp;
- daddr_t cn, bn;
/*
* If they didn't ask for any data, then we are done.
@@ -531,8 +535,7 @@ msdosfs_read(void *v)
if (uio->uio_offset >= dep->de_FileSize)
return (0);
- cn = de_cluster(pmp, uio->uio_offset);
- size = pmp->pm_bpcluster;
+ lbn = de_cluster(pmp, uio->uio_offset);
on = uio->uio_offset & pmp->pm_crbomask;
n = ulmin(pmp->pm_bpcluster - on, uio->uio_resid);
@@ -545,22 +548,31 @@ msdosfs_read(void *v)
if (diff < n)
n = diff;
+ /* convert cluster # to block # if a directory */
+ if (isadir) {
+ error = pcbmap(dep, lbn, &lbn, 0, &blsize);
+ if (error)
+ return (error);
+ }
/*
* If we are operating on a directory file then be sure to
* do i/o with the vnode for the filesystem instead of the
* vnode for the directory.
*/
if (isadir) {
- error = pcbmap(dep, cn, &cn, 0, &size);
- if (error)
- return (error);
- error = bread(pmp->pm_devvp, cn, size, &bp);
+ error = bread(pmp->pm_devvp, lbn, blsize, &bp);
} else {
- bn = de_cn2bn(pmp, cn);
- if (de_cn2off(pmp, cn + 1) >= dep->de_FileSize)
- error = bread(vp, bn, size, &bp);
+ rablock = lbn + 1;
+ rablkno = de_cn2bn(pmp, rablock);
+ if (dep->de_lastr + 1 == lbn &&
+ de_cn2off(pmp, rablock) < dep->de_FileSize)
+ error = breadn(vp, de_cn2bn(pmp, lbn),
+ pmp->pm_bpcluster, &rablkno,
+ &pmp->pm_bpcluster, 1, &bp);
else
- error = bread_cluster(vp, bn, size, &bp);
+ error = bread(vp, de_cn2bn(pmp, lbn),
+ pmp->pm_bpcluster, &bp);
+ dep->de_lastr = lbn;
}
n = min(n, pmp->pm_bpcluster - bp->b_resid);
if (error) {
@@ -589,7 +601,7 @@ msdosfs_write(void *v)
uint32_t osize;
int error = 0;
uint32_t count, lastcn;
- daddr_t cn, bn;
+ daddr_t bn;
struct buf *bp;
int ioflag = ap->a_ioflag;
struct uio *uio = ap->a_uio;
@@ -666,23 +678,20 @@ msdosfs_write(void *v)
lastcn = de_clcount(pmp, osize) - 1;
do {
- croffset = uio->uio_offset & pmp->pm_crbomask;
- cn = de_cluster(pmp, uio->uio_offset);
-
- if (cn > lastcn) {
+ if (de_cluster(pmp, uio->uio_offset) > lastcn) {
error = ENOSPC;
break;
}
- if (croffset == 0 &&
- (de_cluster(pmp, uio->uio_offset + uio->uio_resid) > cn ||
- (uio->uio_offset + uio->uio_resid) >= dep->de_FileSize)) {
+ bn = de_blk(pmp, uio->uio_offset);
+ if ((uio->uio_offset & pmp->pm_crbomask) == 0
+ && (de_blk(pmp, uio->uio_offset + uio->uio_resid) >
de_blk(pmp, uio->uio_offset)
+ || uio->uio_offset + uio->uio_resid >=
dep->de_FileSize)) {
/*
* If either the whole cluster gets written,
* or we write the cluster from its start beyond EOF,
* then no need to read data from disk.
*/
- bn = de_cn2bn(pmp, cn);
bp = getblk(thisvp, bn, pmp->pm_bpcluster, 0, 0);
clrbuf(bp);
/*
@@ -704,10 +713,8 @@ msdosfs_write(void *v)
}
} else {
/*
- * The block we need to write into exists, so
- * read it in.
+ * The block we need to write into exists, so read it
in.
*/
- bn = de_cn2bn(pmp, cn);
error = bread(thisvp, bn, pmp->pm_bpcluster, &bp);
if (error) {
brelse(bp);
@@ -715,6 +722,7 @@ msdosfs_write(void *v)
}
}
+ croffset = uio->uio_offset & pmp->pm_crbomask;
n = ulmin(uio->uio_resid, pmp->pm_bpcluster - croffset);
if (uio->uio_offset + n > dep->de_FileSize) {
dep->de_FileSize = uio->uio_offset + n;
@@ -1747,7 +1755,7 @@ msdosfs_islocked(void *v)
/*
* vp - address of vnode file the file
- * bn - which cluster we are interested in mapping to a filesystem block
number
+ * bn - which cluster we are interested in mapping to a filesystem block
number.
* vpp - returns the vnode for the block special file holding the filesystem
* containing the file of interest
* bnp - address of where to return the filesystem relative block number
@@ -1757,51 +1765,19 @@ msdosfs_bmap(void *v)
{
struct vop_bmap_args *ap = v;
struct denode *dep = VTODE(ap->a_vp);
+ struct msdosfsmount *pmp = dep->de_pmp;
if (ap->a_vpp != NULL)
*ap->a_vpp = dep->de_devvp;
if (ap->a_bnp == NULL)
return (0);
-
- return (msdosfs_bmaparray(ap->a_vp, ap->a_bn, ap->a_bnp, ap->a_runp));
-}
-
-int
-msdosfs_bmaparray(struct vnode *vp, daddr_t cn, daddr_t *bnp, int *runp)
-{
- struct denode *dep = VTODE(vp);
- struct msdosfsmount *pmp = dep->de_pmp;
- struct mount *mp;
- int error, maxrun = 0, run;
- daddr_t daddr;
-
- mp = vp->v_mount;
-
- if (runp) {
+ if (ap->a_runp) {
/*
- * XXX
- * If MAXBSIZE is the largest transfer the disks can handle,
- * we probably want maxrun to be 1 block less so that we
- * don't create a block larger than the device can handle.
+ * Sequential clusters should be counted here.
*/
- *runp = 0;
- maxrun = min(MAXBSIZE / mp->mnt_stat.f_iosize - 1,
- pmp->pm_maxcluster - cn);
+ *ap->a_runp = 0;
}
-
- if ((error = pcbmap(dep, cn, bnp, 0, 0)) != 0)
- return (error);
-
- for (run = 1; run <= maxrun; run++) {
- error = pcbmap(dep, cn + run, &daddr, 0, 0);
- if (error != 0 || (daddr != *bnp + de_cn2bn(pmp, run)))
- break;
- }
-
- if (runp)
- *runp = run - 1;
-
- return (0);
+ return (pcbmap(dep, de_bn2cn(pmp, ap->a_bn), ap->a_bnp, 0, 0));
}
int
