BUG: incorrect PARENT path when removing a folder with files

[Davack]

Hi,
While I was working around AuditD, I encountered an interesting bug when removing a folder the files within the folder are reported with the incorrect PARENT name property.

Repro:
Creating folder with five files inside
mkdir /tmp/test
touch /tmp/1
touch /tmp/2
touch /tmp/3
touch /tmp/4
touch /tmp/5
Removing the folder
rm -rf /tmp/test

AuditD log:

type=SYSCALL msg=audit(1638181912.404:289343): arch=c000003e syscall=263 success=yes exit=0 a0=4 a1=55e90fdb9928 a2=0 a3=0 items=2 ppid=23439 pid=23507 auid=1000 uid=1000 gid=1000 euid=1000 suid=1000 fsuid=1000 egid=1000 sgid=1000 fsgid=1000 tty=pts1 ses=93 comm="rm" exe="/usr/bin/rm" key="mdatp"
type=CWD msg=audit(1638181912.404:289343): cwd="/home/davlevi"
type=PATH msg=audit(1638181912.404:289343): item=0 name="/home/davlevi" inode=917526 dev=fd:00 mode=040775 ouid=1000 ogid=1000 rdev=00:00 nametype=PARENT cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0
type=PATH msg=audit(1638181912.404:289343): item=1 name="3" inode=917532 dev=fd:00 mode=0100664 ouid=1000 ogid=1000 rdev=00:00 nametype=DELETE cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0
type=PROCTITLE msg=audit(1638181912.404:289343): proctitle=726D002D7266002F746D702F74657374
type=SYSCALL msg=audit(1638181912.404:289344): arch=c000003e syscall=263 success=yes exit=0 a0=4 a1=55e90fdc1c58 a2=0 a3=0 items=2 ppid=23439 pid=23507 auid=1000 uid=1000 gid=1000 euid=1000 suid=1000 fsuid=1000 egid=1000 sgid=1000 fsgid=1000 tty=pts1 ses=93 comm="rm" exe="/usr/bin/rm" key="mdatp"
type=CWD msg=audit(1638181912.404:289344): cwd="/home/davlevi"
type=PATH msg=audit(1638181912.404:289344): item=0 name="/home/davlevi" inode=917526 dev=fd:00 mode=040775 ouid=1000 ogid=1000 rdev=00:00 nametype=PARENT cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0
type=PATH msg=audit(1638181912.404:289344): item=1 name="5" inode=917534 dev=fd:00 mode=0100664 ouid=1000 ogid=1000 rdev=00:00 nametype=DELETE cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0
type=PROCTITLE msg=audit(1638181912.404:289344): proctitle=726D002D7266002F746D702F74657374
type=SYSCALL msg=audit(1638181912.404:289345): arch=c000003e syscall=263 success=yes exit=0 a0=4 a1=55e90fdc1d78 a2=0 a3=0 items=2 ppid=23439 pid=23507 auid=1000 uid=1000 gid=1000 euid=1000 suid=1000 fsuid=1000 egid=1000 sgid=1000 fsgid=1000 tty=pts1 ses=93 comm="rm" exe="/usr/bin/rm" key="mdatp"
type=CWD msg=audit(1638181912.404:289345): cwd="/home/davlevi"
type=PATH msg=audit(1638181912.404:289345): item=0 name="/home/davlevi" inode=917526 dev=fd:00 mode=040775 ouid=1000 ogid=1000 rdev=00:00 nametype=PARENT cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0
type=PATH msg=audit(1638181912.404:289345): item=1 name="2" inode=917531 dev=fd:00 mode=0100664 ouid=1000 ogid=1000 rdev=00:00 nametype=DELETE cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0
type=PROCTITLE msg=audit(1638181912.404:289345): proctitle=726D002D7266002F746D702F74657374
type=SYSCALL msg=audit(1638181912.404:289346): arch=c000003e syscall=263 success=yes exit=0 a0=4 a1=55e90fdc1e98 a2=0 a3=0 items=2 ppid=23439 pid=23507 auid=1000 uid=1000 gid=1000 euid=1000 suid=1000 fsuid=1000 egid=1000 sgid=1000 fsgid=1000 tty=pts1 ses=93 comm="rm" exe="/usr/bin/rm" key="mdatp"
type=CWD msg=audit(1638181912.404:289346): cwd="/home/davlevi"
type=PATH msg=audit(1638181912.404:289346): item=0 name="/home/davlevi" inode=917526 dev=fd:00 mode=040775 ouid=1000 ogid=1000 rdev=00:00 nametype=PARENT cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0
type=PATH msg=audit(1638181912.404:289346): item=1 name="1" inode=917528 dev=fd:00 mode=0100664 ouid=1000 ogid=1000 rdev=00:00 nametype=DELETE cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0
type=PROCTITLE msg=audit(1638181912.404:289346): proctitle=726D002D7266002F746D702F74657374
type=SYSCALL msg=audit(1638181912.404:289347): arch=c000003e syscall=263 success=yes exit=0 a0=4 a1=55e90fdc1fb8 a2=0 a3=0 items=2 ppid=23439 pid=23507 auid=1000 uid=1000 gid=1000 euid=1000 suid=1000 fsuid=1000 egid=1000 sgid=1000 fsgid=1000 tty=pts1 ses=93 comm="rm" exe="/usr/bin/rm" key="mdatp"
type=CWD msg=audit(1638181912.404:289347): cwd="/home/davlevi"
type=PATH msg=audit(1638181912.404:289347): item=0 name="/home/davlevi" inode=917526 dev=fd:00 mode=040775 ouid=1000 ogid=1000 rdev=00:00 nametype=PARENT cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0
type=PATH msg=audit(1638181912.404:289347): item=1 name="4" inode=917533 dev=fd:00 mode=0100664 ouid=1000 ogid=1000 rdev=00:00 nametype=DELETE cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0
type=PROCTITLE msg=audit(1638181912.404:289347): proctitle=726D002D7266002F746D702F74657374
type=SYSCALL msg=audit(1638181912.404:289348): arch=c000003e syscall=263 success=yes exit=0 a0=ffffff9c a1=55e90fdb84d0 a2=200 a3=1 items=2 ppid=23439 pid=23507 auid=1000 uid=1000 gid=1000 euid=1000 suid=1000 fsuid=1000 egid=1000 sgid=1000 fsgid=1000 tty=pts1 ses=93 comm="rm" exe="/usr/bin/rm" key="mdatp"
type=CWD msg=audit(1638181912.404:289348): cwd="/home/davlevi"
type=PATH msg=audit(1638181912.404:289348): item=0 name="/tmp/" inode=786434 dev=fd:00 mode=041777 ouid=0 ogid=0 rdev=00:00 nametype=PARENT cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0
type=PATH msg=audit(1638181912.404:289348): item=1 name="/tmp/test" inode=917526 dev=fd:00 mode=040775 ouid=1000 ogid=1000 rdev=00:00 nametype=DELETE cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0
type=PROCTITLE msg=audit(1638181912.404:289348): proctitle=726D002D7266002F746D702F74657374

You can notice that AuditD reporting unlinkat syscall for each of the files but if you look at the PARENT record of each of those deletions you can see that the name property is set to CWD (/home/davlevi) instead of the real parent folder (/tmp/test), you can even see that inode number of the parent (917526) is the correct inode of /tmp/test so only name property is incorrect.

Thanks in advance,
David.

[pcmoore]

Thanks for the report @Davack. What kernel were you running when you observed this?

This is just a guess, but it looks like it is using $CWD in place of the proper directory, "/tmp/test" in your example. Is this something you think you might be able to work on resolving, or are you simply reporting a problem?

[Davack]

uname -a
Linux davlevi-ThinkPad-T490s 5.4.0-90-generic #101-Ubuntu SMP Fri Oct 15 20:00:55 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

Actually, I will have some free time to submit a pull request next week, I will try to track down the relevant code path for this issue :)

[pcmoore]

That's great, thanks @Davack!

[sirotnikov]

We've done some work to analyze the issue.
The issue result from the fact that rm -rf calls unlinkat for all subdirectory files, relative to the /tmp/test/ directory. ). It seems that this issue will affect all file operations relative to open fds (e.g. all *at() syscalls)

The parent directory path is added to the audit context names_list in the context of do_unlinkat() -> filename_parentat() -> audit_inode().

Notice do_unlinkat() calls filename_parentat() with dfd pointing to the parent dir ("/tmp/test/") and the name argument is a relative name equal to "1".

The function filename_parentat() calls path_parentat() which gets the correct struct path parent, the correct inode and fills the last parameter. However, when it calls audit_inode() the name passed is still the relative leaf filename ("1").

Inside auditd_inode(), we arrive to out: label with parent flag set to true, so the assignment

        n->name_len = n->name ? parent_len(n->name->name) : AUDIT_NAME_FULL;

is resolved as folows: parent_len() of "1" returns 0 and n->name_len is set to 0.

Then in audit_log_name() we arrive with path=NULL from audit_log_exit(). The switch on n->name_len lands on case 0:

	/* name was specified as a relative path and the
	* directory component is the cwd
	*/
	if (context->pwd.dentry && context->pwd.mnt)
		audit_log_d_path(ab, " name=", &context->pwd);
	else
		audit_log_format(ab, " name=(null)");
	break;

And we get the pwd printed, instead of the containing directory path.

It seems that auditd needs to make sure it resolves the full parent path either before, or inside audit_inode.

I'm not proficient enough in the the kernel code, to make a PR independently, but I'm willing to do it if assisted.

[pcmoore]

Hi @sirotnikov, thanks for taking the time to debug this and write up your findings!

I'm still thinking a bit about the problem and what some of our options might be, but before I spend too much time on that I wanted to ask if you have a desire to work on a fix for this? You mentioned being willing, but I realize that sometimes there is an important difference between being willing to do something and wanting to do something ;)

I'm very happy to get more people involved in kernel development, so if you want to work on a fix for this I'll do whatever I can to help, just let me know.

However, if you would rather defer to someone else, that's fine too; you've already been a big help in finding the cause of the problem.

[sirotnikov]

Hey @pcmoore
I have the desire, but I'm not great on availability, due to real life commitments.
If you prefer someone that can do it in a timely manner, then it's probably not me.
If you're ok with a slow pace and can provide some hand-holding, then I'll be happy to take this as a learning opportunity.

[pcmoore]

Hi @sirotnikov, no worries, I think we all understand what it is like to juggle multiple commitments.

Why don't you go ahead and work on this, as you are comfortable, and let us know if you have any questions. If we run into a serious or immediate need to get this finished we can always be in touch and perhaps finish up the work collaboratively. How does that sound?

I'd rather get more people involved in audit kernel development than a quick fix ;)

[sirotnikov]

So, let's discuss possible directions for solution.

My feeling is that a correct resolution would be to fix filename_parentat to
a) reconstruct the full path from the parent parameter
b) pass the full parent path to audit_inode
c) make sure that audit_inode with AUDIT_INODE_PARENT flag does what is expected

I'm not familiar enough with the kernel methods, nor with the role of filename_parentat and audit_inode to understand if this suggestion will introduce risk

Also, if there are resources for setting up a dev environment / tests etc, I'd appreciate a reference