Discussion:
[PATCH 1/1] audit: Add new syscalls to the perm=w filter
(too old to reply)
Steve Grubb
2017-10-13 03:24:01 UTC
Permalink
The audit subsystem allows selecting audit events based on watches for
a particular behavior like writing to a file. A lot of syscalls have
been added without updating the list. This patch adds 2 syscalls to the
write filters: fallocate and renameat2.

Signed-off-by: sgrubb <***@redhat.com>
---
include/asm-generic/audit_dir_write.h | 4 ++++
include/asm-generic/audit_write.h | 3 +++
2 files changed, 7 insertions(+)

diff --git a/include/asm-generic/audit_dir_write.h b/include/asm-generic/audit_dir_write.h
index 7b61db4fe72b..d9a53eaeea35 100644
--- a/include/asm-generic/audit_dir_write.h
+++ b/include/asm-generic/audit_dir_write.h
@@ -30,3 +30,7 @@ __NR_renameat,
__NR_linkat,
__NR_symlinkat,
#endif
+#ifdef __NR_renameat2
+__NR_renameat2,
+#endif
+
diff --git a/include/asm-generic/audit_write.h b/include/asm-generic/audit_write.h
index 274575d7129f..4fa65816acd3 100644
--- a/include/asm-generic/audit_write.h
+++ b/include/asm-generic/audit_write.h
@@ -19,3 +19,6 @@ __NR_ftruncate64,
#ifdef __NR_bind
__NR_bind, /* bind can affect fs object only in one way... */
#endif
+#ifdef __NR_fallocate
+__NR_fallocate,
+#endif
--
2.13.6
Richard Guy Briggs
2017-10-16 19:00:07 UTC
Permalink
Post by Steve Grubb
The audit subsystem allows selecting audit events based on watches for
a particular behavior like writing to a file. A lot of syscalls have
been added without updating the list. This patch adds 2 syscalls to the
write filters: fallocate and renameat2.
---
include/asm-generic/audit_dir_write.h | 4 ++++
include/asm-generic/audit_write.h | 3 +++
2 files changed, 7 insertions(+)
diff --git a/include/asm-generic/audit_dir_write.h b/include/asm-generic/audit_dir_write.h
index 7b61db4fe72b..d9a53eaeea35 100644
--- a/include/asm-generic/audit_dir_write.h
+++ b/include/asm-generic/audit_dir_write.h
@@ -30,3 +30,7 @@ __NR_renameat,
__NR_linkat,
__NR_symlinkat,
#endif
+#ifdef __NR_renameat2
+__NR_renameat2,
+#endif
+
diff --git a/include/asm-generic/audit_write.h b/include/asm-generic/audit_write.h
index 274575d7129f..4fa65816acd3 100644
--- a/include/asm-generic/audit_write.h
+++ b/include/asm-generic/audit_write.h
@@ -19,3 +19,6 @@ __NR_ftruncate64,
#ifdef __NR_bind
__NR_bind, /* bind can affect fs object only in one way... */
#endif
+#ifdef __NR_fallocate
+__NR_fallocate,
+#endif
--
2.13.6
- RGB

--
Richard Guy Briggs <***@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
Richard Guy Briggs
2017-10-16 19:06:20 UTC
Permalink
Post by Steve Grubb
The audit subsystem allows selecting audit events based on watches for
a particular behavior like writing to a file. A lot of syscalls have
been added without updating the list. This patch adds 2 syscalls to the
write filters: fallocate and renameat2.
Please add a link to the issue number in the body of the patch description:

See: https://github.com/linux-audit/audit-kernel/issues/67
Post by Steve Grubb
---
include/asm-generic/audit_dir_write.h | 4 ++++
include/asm-generic/audit_write.h | 3 +++
2 files changed, 7 insertions(+)
diff --git a/include/asm-generic/audit_dir_write.h b/include/asm-generic/audit_dir_write.h
index 7b61db4fe72b..d9a53eaeea35 100644
--- a/include/asm-generic/audit_dir_write.h
+++ b/include/asm-generic/audit_dir_write.h
@@ -30,3 +30,7 @@ __NR_renameat,
__NR_linkat,
__NR_symlinkat,
#endif
+#ifdef __NR_renameat2
+__NR_renameat2,
+#endif
+
diff --git a/include/asm-generic/audit_write.h b/include/asm-generic/audit_write.h
index 274575d7129f..4fa65816acd3 100644
--- a/include/asm-generic/audit_write.h
+++ b/include/asm-generic/audit_write.h
@@ -19,3 +19,6 @@ __NR_ftruncate64,
#ifdef __NR_bind
__NR_bind, /* bind can affect fs object only in one way... */
#endif
+#ifdef __NR_fallocate
+__NR_fallocate,
+#endif
--
2.13.6
- RGB
- RGB

--
Richard Guy Briggs <***@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
Paul Moore
2017-10-16 19:15:03 UTC
Permalink
Post by Richard Guy Briggs
Post by Steve Grubb
The audit subsystem allows selecting audit events based on watches for
a particular behavior like writing to a file. A lot of syscalls have
been added without updating the list. This patch adds 2 syscalls to the
write filters: fallocate and renameat2.
See: https://github.com/linux-audit/audit-kernel/issues/67
FWIW, I don't really care if the upstream issue is included in the
submitted patch; if you want to include it - great, if you don't -
that's fine too. The commit description needs to stand on its own,
regardless of any external issue trackers, mailing lists, etc.

I'm guessing based on your constant reminders that Steve has gotten
the message at this point that you would really prefer he added the
issue tracker numbers; I'm not sure you need to keep mentioning it. ;)
Post by Richard Guy Briggs
Post by Steve Grubb
---
include/asm-generic/audit_dir_write.h | 4 ++++
include/asm-generic/audit_write.h | 3 +++
2 files changed, 7 insertions(+)
diff --git a/include/asm-generic/audit_dir_write.h b/include/asm-generic/audit_dir_write.h
index 7b61db4fe72b..d9a53eaeea35 100644
--- a/include/asm-generic/audit_dir_write.h
+++ b/include/asm-generic/audit_dir_write.h
@@ -30,3 +30,7 @@ __NR_renameat,
__NR_linkat,
__NR_symlinkat,
#endif
+#ifdef __NR_renameat2
+__NR_renameat2,
+#endif
+
diff --git a/include/asm-generic/audit_write.h b/include/asm-generic/audit_write.h
index 274575d7129f..4fa65816acd3 100644
--- a/include/asm-generic/audit_write.h
+++ b/include/asm-generic/audit_write.h
@@ -19,3 +19,6 @@ __NR_ftruncate64,
#ifdef __NR_bind
__NR_bind, /* bind can affect fs object only in one way... */
#endif
+#ifdef __NR_fallocate
+__NR_fallocate,
+#endif
--
2.13.6
- RGB
- RGB
--
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
--
Linux-audit mailing list
https://www.redhat.com/mailman/listinfo/linux-audit
--
paul moore
www.paul-moore.com
Steve Grubb
2017-10-16 20:47:13 UTC
Permalink
Post by Paul Moore
Post by Richard Guy Briggs
Post by Steve Grubb
The audit subsystem allows selecting audit events based on watches for
a particular behavior like writing to a file. A lot of syscalls have
been added without updating the list. This patch adds 2 syscalls to the
write filters: fallocate and renameat2.
See: https://github.com/linux-audit/audit-kernel/issues/67
FWIW, I don't really care if the upstream issue is included in the
submitted patch; if you want to include it - great, if you don't -
that's fine too. The commit description needs to stand on its own,
regardless of any external issue trackers, mailing lists, etc.
I honestly don't know what the protocol is here. Should I resend the patch
with that or is that fixed up in the merge process? The reason I ask is on the
user space side I never make anyone resend a patch unless its grossly wrong or
incomplete. I just fix it. But that's what I do and not everyone works that
way.
Post by Paul Moore
I'm guessing based on your constant reminders that Steve has gotten
the message at this point that you would really prefer he added the
issue tracker numbers;
I get it, but in the case of the bind/unbind I didn't even know there was a
tracker.

-Steve
Paul Moore
2017-10-17 01:20:11 UTC
Permalink
Post by Steve Grubb
Post by Paul Moore
Post by Richard Guy Briggs
Post by Steve Grubb
The audit subsystem allows selecting audit events based on watches for
a particular behavior like writing to a file. A lot of syscalls have
been added without updating the list. This patch adds 2 syscalls to the
write filters: fallocate and renameat2.
See: https://github.com/linux-audit/audit-kernel/issues/67
FWIW, I don't really care if the upstream issue is included in the
submitted patch; if you want to include it - great, if you don't -
that's fine too. The commit description needs to stand on its own,
regardless of any external issue trackers, mailing lists, etc.
I honestly don't know what the protocol is here. Should I resend the patch
with that or is that fixed up in the merge process? The reason I ask is on the
user space side I never make anyone resend a patch unless its grossly wrong or
incomplete. I just fix it. But that's what I do and not everyone works that
way.
It really depends on the situation, there is no strict rule to follow,
although if I'm expecting you to respin a patch I'll let you know. In
the comment above I said that I didn't care either way (about the GH
issue), that means you don't need to worry about it. I also said I'd
fixup your sign-off line when I apply the patch, there is no need to
respin, although you do need to fix that from this point on (future
new patches as well as any I ask you to respin).

In general, heavy editing by the maintainer is discouraged; exceptions
are made for merge fuzz, trivial fixes, agreed upon tweaks, etc.
There is also a bit of a human factor if we are truly honest, but I
try to minimize that as much as possible (am I in a good mood and
wiling to fix the code? has the contributor annoyed me lately?
etc.). At the end of the day, every maintainer is a bit different.
--
paul moore
www.paul-moore.com
Paul Moore
2017-10-16 19:10:59 UTC
Permalink
Post by Steve Grubb
The audit subsystem allows selecting audit events based on watches for
a particular behavior like writing to a file. A lot of syscalls have
been added without updating the list. This patch adds 2 syscalls to the
write filters: fallocate and renameat2.
---
include/asm-generic/audit_dir_write.h | 4 ++++
include/asm-generic/audit_write.h | 3 +++
2 files changed, 7 insertions(+)
FWIW, I expect that this syscall list is almost always going to be out
of date; it's just the way this feature is designed. That doesn't
mean I'm not going to merge fixes, I just want to make sure
expectations are set accordingly.

Before I merge this Steve, can you explain why fallocate() should be
on the write list? It doesn't actually write any user data to disk,
it actually doesn't write anything, all it does is play with the
amount of space allocated for the given fd on the storage device. I
don't really care either way, this just struck me as odd and I want to
make sure you have a good reason (hint: add it to the patch
description).
Post by Steve Grubb
diff --git a/include/asm-generic/audit_dir_write.h b/include/asm-generic/audit_dir_write.h
index 7b61db4fe72b..d9a53eaeea35 100644
--- a/include/asm-generic/audit_dir_write.h
+++ b/include/asm-generic/audit_dir_write.h
@@ -30,3 +30,7 @@ __NR_renameat,
__NR_linkat,
__NR_symlinkat,
#endif
+#ifdef __NR_renameat2
+__NR_renameat2,
+#endif
+
diff --git a/include/asm-generic/audit_write.h b/include/asm-generic/audit_write.h
index 274575d7129f..4fa65816acd3 100644
--- a/include/asm-generic/audit_write.h
+++ b/include/asm-generic/audit_write.h
@@ -19,3 +19,6 @@ __NR_ftruncate64,
#ifdef __NR_bind
__NR_bind, /* bind can affect fs object only in one way... */
#endif
+#ifdef __NR_fallocate
+__NR_fallocate,
+#endif
--
paul moore
www.paul-moore.com
Paul Moore
2017-10-16 19:18:34 UTC
Permalink
Post by Paul Moore
Post by Steve Grubb
The audit subsystem allows selecting audit events based on watches for
a particular behavior like writing to a file. A lot of syscalls have
been added without updating the list. This patch adds 2 syscalls to the
write filters: fallocate and renameat2.
---
include/asm-generic/audit_dir_write.h | 4 ++++
include/asm-generic/audit_write.h | 3 +++
2 files changed, 7 insertions(+)
FWIW, I expect that this syscall list is almost always going to be out
of date; it's just the way this feature is designed. That doesn't
mean I'm not going to merge fixes, I just want to make sure
expectations are set accordingly.
Before I merge this Steve, can you explain why fallocate() should be
on the write list? It doesn't actually write any user data to disk,
it actually doesn't write anything, all it does is play with the
amount of space allocated for the given fd on the storage device. I
don't really care either way, this just struck me as odd and I want to
make sure you have a good reason (hint: add it to the patch
description).
Oh, one more thing; it's administrative and not tied to a particular
patch ... there is no need to add write "PATCH 1/1" when there is just
one patch, a simple "PATCH" is sufficient. The extra "1/1" just adds
a bit of extra work as I need to clean it up before merging; it's not
a big deal, but if I still see you doing it a month from now I may
have to get a bit salty ;)
--
paul moore
www.paul-moore.com
Paul Moore
2017-10-16 19:35:49 UTC
Permalink
Post by Paul Moore
Post by Paul Moore
Post by Steve Grubb
The audit subsystem allows selecting audit events based on watches for
a particular behavior like writing to a file. A lot of syscalls have
been added without updating the list. This patch adds 2 syscalls to the
write filters: fallocate and renameat2.
One more one more thing ;)

You are supposed to use your "Full Name" and not a username, see
Documentation/process/5.Posting.rst
for more information. I'm going to go ahead and substitute "Steve
Grubb" because that is how you are sending your emails, but please
correct this in the future; sign-off lines are very important.
Post by Paul Moore
Post by Paul Moore
Post by Steve Grubb
---
include/asm-generic/audit_dir_write.h | 4 ++++
include/asm-generic/audit_write.h | 3 +++
2 files changed, 7 insertions(+)
FWIW, I expect that this syscall list is almost always going to be out
of date; it's just the way this feature is designed. That doesn't
mean I'm not going to merge fixes, I just want to make sure
expectations are set accordingly.
Before I merge this Steve, can you explain why fallocate() should be
on the write list? It doesn't actually write any user data to disk,
it actually doesn't write anything, all it does is play with the
amount of space allocated for the given fd on the storage device. I
don't really care either way, this just struck me as odd and I want to
make sure you have a good reason (hint: add it to the patch
description).
Oh, one more thing; it's administrative and not tied to a particular
patch ... there is no need to add write "PATCH 1/1" when there is just
one patch, a simple "PATCH" is sufficient. The extra "1/1" just adds
a bit of extra work as I need to clean it up before merging; it's not
a big deal, but if I still see you doing it a month from now I may
have to get a bit salty ;)
--
paul moore
www.paul-moore.com
Steve Grubb
2017-10-16 20:31:09 UTC
Permalink
Post by Paul Moore
Post by Steve Grubb
The audit subsystem allows selecting audit events based on watches for
a particular behavior like writing to a file. A lot of syscalls have
been added without updating the list. This patch adds 2 syscalls to the
write filters: fallocate and renameat2.
---
include/asm-generic/audit_dir_write.h | 4 ++++
include/asm-generic/audit_write.h | 3 +++
2 files changed, 7 insertions(+)
FWIW, I expect that this syscall list is almost always going to be out
of date; it's just the way this feature is designed. That doesn't
mean I'm not going to merge fixes, I just want to make sure
expectations are set accordingly.
I understand...but we are years behind. I just wanted to close the gap on a
couple obvious syscalls since everyone else is busy with more important bugs.
I have not done an indepth review, so there may still be others. For example,
do we consider time stamp of a file one of its attributes? If so, then
utimensat may need to be brought in. I also have no idea what to make of
name_to_handle_at and open_by_handle_at.
Post by Paul Moore
Before I merge this Steve, can you explain why fallocate() should be
on the write list?
Its a whole lot like ftruncate.

$ touch gh
$ ls -l gh
-rw-rw-r--. 1 sgrubb sgrubb 0 Oct 16 16:21 gh

strace fallocate -o 0 -l 1024 -z gh
<snip>
open("gh", O_RDWR) = 3
fallocate(3, FALLOC_FL_ZERO_RANGE, 0, 1024) = 0
fsync(3) = 0
close(3) = 0
<snip>
$ ls -l gh
-rw-rw-r--. 1 sgrubb sgrubb 1024 Oct 16 16:21 gh

File went from 0 bytes to 1024 bytes.
Post by Paul Moore
It doesn't actually write any user data to disk, it actually doesn't write
anything, all it does is play with the amount of space allocated for the
given fd on the storage device.
It has a bunch of flags which can zeroize holes or extents.
Post by Paul Moore
I don't really care either way, this just struck me as odd and I want to
make sure you have a good reason (hint: add it to the patch
description).
Understandable. But its close enough to ftruncate that I think it qualifies.

-Steve
Post by Paul Moore
Post by Steve Grubb
diff --git a/include/asm-generic/audit_dir_write.h
b/include/asm-generic/audit_dir_write.h index 7b61db4fe72b..d9a53eaeea35
100644
--- a/include/asm-generic/audit_dir_write.h
+++ b/include/asm-generic/audit_dir_write.h
@@ -30,3 +30,7 @@ __NR_renameat,
__NR_linkat,
__NR_symlinkat,
#endif
+#ifdef __NR_renameat2
+__NR_renameat2,
+#endif
+
diff --git a/include/asm-generic/audit_write.h
b/include/asm-generic/audit_write.h index 274575d7129f..4fa65816acd3
100644
--- a/include/asm-generic/audit_write.h
+++ b/include/asm-generic/audit_write.h
@@ -19,3 +19,6 @@ __NR_ftruncate64,
#ifdef __NR_bind
__NR_bind, /* bind can affect fs object only in one way... */
#endif
+#ifdef __NR_fallocate
+__NR_fallocate,
+#endif
Paul Moore
2017-10-17 01:05:23 UTC
Permalink
Post by Steve Grubb
Post by Paul Moore
Post by Steve Grubb
The audit subsystem allows selecting audit events based on watches for
a particular behavior like writing to a file. A lot of syscalls have
been added without updating the list. This patch adds 2 syscalls to the
write filters: fallocate and renameat2.
---
include/asm-generic/audit_dir_write.h | 4 ++++
include/asm-generic/audit_write.h | 3 +++
2 files changed, 7 insertions(+)
FWIW, I expect that this syscall list is almost always going to be out
of date; it's just the way this feature is designed. That doesn't
mean I'm not going to merge fixes, I just want to make sure
expectations are set accordingly.
I understand...but we are years behind. I just wanted to close the gap on a
couple obvious syscalls since everyone else is busy with more important bugs.
No worries, I'm perfectly fine with chipping away at things, I just
wanted to make sure that people aren't expecting this to be current.
The way it's designed I can almost guarantee it will always lag.
Post by Steve Grubb
Post by Paul Moore
I don't really care either way, this just struck me as odd and I want to
make sure you have a good reason (hint: add it to the patch
description).
Understandable. But its close enough to ftruncate that I think it qualifies.
That's fine, I didn't feel very strongly about it either way. I'll
merge this tomorrow when I'm back in front of the system with my audit
kernel repo.
--
paul moore
www.paul-moore.com
Paul Moore
2017-10-17 14:11:36 UTC
Permalink
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
Post by Steve Grubb
The audit subsystem allows selecting audit events based on watches for
a particular behavior like writing to a file. A lot of syscalls have
been added without updating the list. This patch adds 2 syscalls to the
write filters: fallocate and renameat2.
---
include/asm-generic/audit_dir_write.h | 4 ++++
include/asm-generic/audit_write.h | 3 +++
2 files changed, 7 insertions(+)
FWIW, I expect that this syscall list is almost always going to be out
of date; it's just the way this feature is designed. That doesn't
mean I'm not going to merge fixes, I just want to make sure
expectations are set accordingly.
I understand...but we are years behind. I just wanted to close the gap on a
couple obvious syscalls since everyone else is busy with more important bugs.
No worries, I'm perfectly fine with chipping away at things, I just
wanted to make sure that people aren't expecting this to be current.
The way it's designed I can almost guarantee it will always lag.
Post by Steve Grubb
Post by Paul Moore
I don't really care either way, this just struck me as odd and I want to
make sure you have a good reason (hint: add it to the patch
description).
Understandable. But its close enough to ftruncate that I think it qualifies.
That's fine, I didn't feel very strongly about it either way. I'll
merge this tomorrow when I'm back in front of the system with my audit
kernel repo.
Merged into audit/next.
--
paul moore
www.paul-moore.com
Loading...