Page MenuHomeFreeBSD

msdosfs: drop DE_RENAME
AbandonedPublic

Authored by trasz on Jul 31 2021, 6:12 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Oct 24, 4:10 AM
Unknown Object (File)
Oct 7 2024, 1:20 PM
Unknown Object (File)
Oct 6 2024, 2:47 PM
Unknown Object (File)
Oct 3 2024, 11:59 AM
Unknown Object (File)
Oct 3 2024, 9:55 AM
Unknown Object (File)
Oct 1 2024, 2:24 PM
Unknown Object (File)
Oct 1 2024, 8:38 AM
Unknown Object (File)
Sep 16 2024, 2:21 AM
Subscribers

Details

Reviewers
kib
Summary

Clearing the DE_RENAME flag, done near the end of msdosfs_rename(),
can result in overwriting freed memory: since fvp is not locked
at that point, ip might no longer be valid.

Instead of trying to invent a safe way of resetting the flag
(https://reviews.freebsd.org/D27337), drop it altogether.
The cases it's supposed to prevent from shouldn't break anything.

Note that this is relatively untested, as msdosfs_rename() is already
broken in a different way; see https://bugs.freebsd.org/257522.
I'm still interested in feedback, in particular about the approach.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 40779
Build 37668: arc lint + arc unit

Event Timeline

trasz requested review of this revision.Jul 31 2021, 6:12 PM

What cases is it supposed to prevent?

In D31366#706899, @imp wrote:

What cases is it supposed to prevent?

It all started with NetApp discovering that this line:

ip->de_flag &= ~DE_RENAME

... can result in overwriting freed memory, if the vnode the ip is attached to gets reclaimed. Then kib@ noticed that the fix was still racy, but more importantly, the DE_RENAME flag doesn't seem to be useful anyway: it protects from a case which shouldn't break anything, except for the calls to panic(9) when the code detects the situation.

I started the proper fix for msdosfs_rename(). It will take a while before I have a patch that can be even tested.