On 17 June 2014 18:09, Tobias Stoeckmann <[email protected]> wrote: > Hi, > > fsck_msdos checks for linked cluster chains, which means that two chains > cross each other at the same cluster. If 1 links to 3 and 2 links to 3, > the cluster chains starting at 1 and 2 are linked. > > This error condition can be fixed during phase 2, either one or both > chains are dropped or the first gets truncated. > > Now _if_ the chain gets truncated, the length of the cluster head still > contains the old value, which means that length can be longer than the > actual chain. > > This becomes an issue in phase 3, in which the length variable is > accessed again to retrieve the "physical size" of a file. If this > physical size is longer than the size in the directory record, > fsck_msdos offers the possibility to truncate the chain. > > And that will lead to an out of boundary access of the fat array. I > did not see a crash on my i386 system, but wrong is wrong anyway. If > you want to test on your system, you can retrieve a modified ISO image > from my website: http://stoeckmann.org/openbsd/truncate.iso > > In this ISO file, two chains cross. After truncation, chain will be too > short compared to the directory entry's size (FILE1.TXT). > Yet fsck_msdos tries to drop superfluous clusters. > > # vnconfig vnd0c truncate.iso > # fsck_msdos vnd0c > ** /dev/rvnd0c (vnd0c) > ** Phase 1 - Read and Compare FATs > ** Phase 2 - Check Cluster Chains > Cluster chains starting at 8 and 18 are linked at cluster 5 > Clear chain starting at 8? [Fyn] n > Truncate? [Fyn] y > Clear chain starting at 18? [Fyn] n > ** Phase 3 - Check Directories > /FILE1.TXT has too many clusters allocated > Drop superfluous clusters? [Fyn] y > /FILE2.TXT has too many clusters allocated > Drop superfluous clusters? [Fyn] y > ** Phase 4 - Check for Lost Files > Lost cluster chain at cluster 9 > 9 Cluster(s) lost > Reconnect? [Fyn] n > Clear? [Fyn] y > 3 files, 36 free (9 clusters) > > It's hard to see any form of error with these file systems, but you > will notice difference in behaviour if you stop fsck_msdos after > fixing the linked clusters: > > # fsck_msdos vnd0c > ** /dev/rvnd0c (vnd0c) > ** Phase 1 - Read and Compare FATs > ** Phase 2 - Check Cluster Chains > Cluster chains starting at 8 and 18 are linked at cluster 5 > Clear chain starting at 8? [Fyn] n > Truncate? [Fyn] y > Clear chain starting at 18? [Fyn] n > ** Phase 3 - Check Directories > /FILE1.TXT has too many clusters allocated > Drop superfluous clusters? [Fyn] ^C > # fsck_msdos vnd0c > ** /dev/rvnd0c (vnd0c) > ** Phase 1 - Read and Compare FATs > ** Phase 2 - Check Cluster Chains > ** Phase 3 - Check Directories > size of /FILE1.TXT is 8192, should at most be 4096 > Truncate? [Fyn] ^C > > Instead of dropping superfluous clusters, it actually wants to fix > the directory entry that is now "too large". > > > The diff verifies that the length is always kept up to date after > truncation. > > > Tobias > > Index: dir.c > =================================================================== > RCS file: /cvs/src/sbin/fsck_msdos/dir.c,v > retrieving revision 1.22 > diff -u -p -r1.22 dir.c > --- dir.c 16 Jun 2014 18:33:33 -0000 1.22 > +++ dir.c 17 Jun 2014 21:50:57 -0000 > @@ -374,7 +374,7 @@ checksize(struct bootblock *boot, struct > /* > * Check size on ordinary files > */ > - int32_t physicalSize; > + u_int32_t physicalSize; > > if (dir->head == CLUST_FREE) > physicalSize = 0; > @@ -400,12 +400,16 @@ checksize(struct bootblock *boot, struct > fullpath(dir)); > if (ask(1, "Drop superfluous clusters")) { > cl_t cl; > - u_int32_t sz = 0; > + u_int32_t len, sz; > > - for (cl = dir->head; (sz += boot->ClusterSize) < > dir->size;) > + len = sz = 0; > + for (cl = dir->head; (sz += boot->ClusterSize) < > dir->size;) { > cl = fat[cl].next; > + len++; > + } > clearchain(boot, fat, fat[cl].next); > fat[cl].next = CLUST_EOF; > + fat[dir->head].length = len; > return (FSFATMOD); > } else > return (FSERROR); > Index: fat.c > =================================================================== > RCS file: /cvs/src/sbin/fsck_msdos/fat.c,v > retrieving revision 1.23 > diff -u -p -r1.23 fat.c > --- fat.c 16 Jun 2014 18:33:33 -0000 1.23 > +++ fat.c 17 Jun 2014 21:50:58 -0000 > @@ -302,11 +302,20 @@ clearchain(struct bootblock *boot, struc > int > tryclear(struct bootblock *boot, struct fatEntry *fat, cl_t head, cl_t > *trunc) > { > + u_int len; > + cl_t p; > + > if (ask(0, "Clear chain starting at %u", head)) { > clearchain(boot, fat, head); > return FSFATMOD; > } else if (ask(0, "Truncate")) { > *trunc = CLUST_EOF; > + len = 0; > + for (p = head; p >= CLUST_FIRST && p < boot->NumClusters; > + p = fat[p].next) { > + len++; > + } > + fat[head].length = len; > return FSFATMOD; > } else > return FSERROR; >
Makes sense to me. ok krw@ .... Ken
