Page MenuHomeFreeBSD

msdosfs: drop DE_RENAME
AbandonedPublic

Authored by trasz on Jul 31 2021, 6:12 PM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 18 2024, 6:04 PM
Unknown Object (File)
Dec 21 2023, 10:03 PM
Unknown Object (File)
Dec 20 2023, 3:07 AM
Unknown Object (File)
Oct 10 2023, 7:21 AM
Unknown Object (File)
Jun 20 2023, 7:01 AM
Unknown Object (File)
May 15 2023, 5:21 AM
Unknown Object (File)
Apr 26 2023, 4:12 AM
Unknown Object (File)
Mar 31 2023, 8:02 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.