Page MenuHomeFreeBSD

fsck_msdosfs: truncate directory entry when the head pointer is invalid.
ClosedPublic

Authored by delphij on Oct 27 2021, 9:56 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mar 12 2024, 7:14 PM
Unknown Object (File)
Mar 11 2024, 7:26 PM
Unknown Object (File)
Mar 11 2024, 7:22 PM
Unknown Object (File)
Mar 8 2024, 3:22 AM
Unknown Object (File)
Feb 6 2024, 7:16 AM
Unknown Object (File)
Jan 4 2024, 10:40 AM
Unknown Object (File)
Jan 4 2024, 9:43 AM
Unknown Object (File)
Jan 4 2024, 1:53 AM
Subscribers

Details

Summary

As far as we know, there is no FAT implementation that supported hard
links, and our msdosfs driver assumed one cluster chain is only
referenced by one directory entry and clears it out when the file is
deleted. On the other hand, the current code would proceed with
checkchain() when the directory entry's head cluster is a valid numbered
cluster without checking if it was a valid head node of a cluster chain.

So if the cluster do not being a chain (e.g. CLUST_FREE, CLUST_BAD),
or was already referenced by another directory entry, this would
trigger an assertion in check_chain() at a later time.

Fix this by giving the user an option to truncate the directory entry
when the head cluster is an invalid cluster, an visited head node,
or not a head node.

Reported by: NetApp (kevans@)

Test Plan

run fsck_msdosfs on actual file system

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Aha, I /think/ that makes a lot more sense without causing any collateral damage. I'm re-running it through the test setup I have to confirm we're not hitting any other landmines... this seems to be an image being baked in the test process, but unfortunately I'm not sure how it's being produced to offer up a test image to reproduce against.

I think this corresponds with this part of the artist formerly known as checkfat():

                /* find next untravelled chain */                               
                if (fat[head].head != head)                                     
                        continue;                                               
                                                                                
                /* follow the chain to its end (hopefully) */                   
                for (len = fat[head].length, p = head;                          
                     (n = fat[p].next) >= CLUST_FIRST && n < boot->NumClusters; 
                     p = n)                                                     
                        if (fat[n].head != head || len-- < 2)                   
                                break;                                          
                if (n >= CLUST_EOFS)                                            
                        continue;                                               
                                                                                
                if (n == CLUST_FREE || n >= CLUST_RSRVD) {                      
                        pwarn("Cluster chain starting at %u ends with cluster marked %s\n",
                              head, rsrvdcltype(n));                            
clear:                                                                          
                        ret |= tryclear(boot, fat, head, &fat[p].next);         
                        continue;                                               
                }

pass 1 wouldn't have traversed the chain because head->next == CLUST_FREE, so it gets hit here in pass2 and takes the tryclear() route.

No objection from me, looks reasonable.

This appears to match the way it was previously handled, and it works for NetApp's purposes. Thanks!

This revision is now accepted and ready to land.Nov 4 2021, 4:57 AM