Re: [SECURITY] Re: PATCH: rm -rf and readdir bug in coreutils-6.7

[Paul Eggert]

Mikulas Patocka <address@hidden> writes:

> They have 32-bit ino_t but 64-bit stat64.st_ino
> (stat64 has unsigned long long st_ino) --- although kernel has the
> deficiency that it can pass only 32-bit values to it --- BTW. can you
> accept that fact that sizeof(st_ino) > sizeof(ino_t) in coreutils?
> (although it violates standard).

No, that breaks coreutils.  Such operating systems are obviously
broken, anyway.  If a program uses the large-file interface and inode
numbers are 64 bits, then ino_t should be 64 bits.

It should be fairly easy to work around such a bug by discarding the
system-supplied ino_t and replacing it, e.g., with something like this
in user code:

#ifdef INO_T_REPLACEMENT_TYPE
# define ino_t INO_T_REPLACEMENT_TYPE
#endif

If you can write some Autoconfish stuff to #define
INO_T_REPLACEMENT_TYPE in config.h, I'm sure we'd accept it.  This
would be a simple fix.

But a better fix, obviously, is to fix those broken operating systems,
whatever they are.  Having an incorrect ino_t breaks lots of programs,
not just coreutils.

One thought: perhaps we should have a coreutils test that st_ino and
ino_t are the same type?  We could put something like this into the
coreutils 'configure' file:

    struct stat st;
    ino_t *p = &st.st_ino;

This would catch the broken operating systems at compile time, rather
than at run time.  Perhaps you can make this part of the
INO_T_REPLACEMENT_TYPE test.

>>> Another possibility to fix it (without rewriting the whole algorithm)
>>> would be to compare not only st_ino and st_dev but also st_uid and st_gid
>>> in macro SAME_INODE

I'd rather not head this direction.  Aside from the performance
issues, it means an attacker can mess up your 'rm -fr' by chgrp'ing
one of your files while rm is operating.  Let's fix the real problem
instead.