On 06/12/17 21:41, Stefan Fritsch wrote: > 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.
Yes, this patch fixes the problem. Kind regards, Martijn Rijkeboer > > 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 >