On Sun, Aug 7, 2016 at 8:17 PM, Joseph Burt <[email protected]> wrote: > On Sun, Aug 7, 2016 at 5:02 AM, Allan McRae <[email protected]> wrote: >> It would be better to fix _alpm_access and fix all occurrences of this >> rather than replace this one instance. >> > You're right of course, this is just the smallest change that would > correct the bug. Since _alpm_access can't tell from the W_OK mode > whether we mean to open for write (follow symlinks) or unlink (don't), > it's not enough. Thoughts on new util functions like > "_alpm_can_unlink"? > >> Note AT_SYMLINK_NOFOLLOW is not available on OSX. I believe pacman >> still builds there currently, but I can't remember if we decided not to >> support it... >> > Ok, so that means using lstat instead of faccessat. > > There's also a TOCTOU bug here just for checking at all. Access or > lstat can't guarantee that open or unlink will succeed, so why bother? > Those failing should be dealt with properly anyway. The question is, > if a file fails to unlink, after logging it, should you remove the > package from the db, leaving orphaned files? Or leave it, but now > possibly missing files? Or keep FDs open for every file in a package > until all unlinks succeed, and if one fails, try to undo by copying > back the already unlnked files? > This is a design decision...
Allan, any thoughts on what you'd like here? I'm currently working around this bug by removing the can_remove_file check entirely.
