| Message ID | 1081821010c124fe4e35984ec3dac1654453bb7c.1521179281.git.rgb@redhat.com |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | audit: implement container id | expand |
On Friday, March 16, 2018 5:00:40 AM EDT Richard Guy Briggs wrote: > Add support for reading the container ID from the proc filesystem. I think this could be useful in general. Please consider this to be part of the full patch set and not something merely used to debug the patches. -Steve > This is a read from the proc entry of the form /proc/PID/containerid > where PID is the process ID of the task whose container ID is sought. > > The read expects up to a u64 value (unset: 18446744073709551615). > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > --- > fs/proc/base.c | 20 ++++++++++++++++++-- > 1 file changed, 18 insertions(+), 2 deletions(-) > > diff --git a/fs/proc/base.c b/fs/proc/base.c > index 6ce4fbe..f66d1e2 100644 > --- a/fs/proc/base.c > +++ b/fs/proc/base.c > @@ -1300,6 +1300,21 @@ static ssize_t proc_sessionid_read(struct file * > file, char __user * buf, .llseek = generic_file_llseek, > }; > > +static ssize_t proc_containerid_read(struct file *file, char __user *buf, > + size_t count, loff_t *ppos) > +{ > + struct inode *inode = file_inode(file); > + struct task_struct *task = get_proc_task(inode); > + ssize_t length; > + char tmpbuf[TMPBUFLEN*2]; > + > + if (!task) > + return -ESRCH; > + length = scnprintf(tmpbuf, TMPBUFLEN*2, "%llu", > audit_get_containerid(task)); + put_task_struct(task); > + return simple_read_from_buffer(buf, count, ppos, tmpbuf, length); > +} > + > static ssize_t proc_containerid_write(struct file *file, const char __user > *buf, size_t count, loff_t *ppos) > { > @@ -1330,6 +1345,7 @@ static ssize_t proc_containerid_write(struct file > *file, const char __user *buf, } > > static const struct file_operations proc_containerid_operations = { > + .read = proc_containerid_read, > .write = proc_containerid_write, > .llseek = generic_file_llseek, > }; > @@ -2996,7 +3012,7 @@ static int proc_pid_patch_state(struct seq_file *m, > struct pid_namespace *ns, #ifdef CONFIG_AUDITSYSCALL > REG("loginuid", S_IWUSR|S_IRUGO, proc_loginuid_operations), > REG("sessionid", S_IRUGO, proc_sessionid_operations), > - REG("containerid", S_IWUSR, proc_containerid_operations), > + REG("containerid", S_IWUSR|S_IRUSR, proc_containerid_operations), > #endif > #ifdef CONFIG_FAULT_INJECTION > REG("make-it-fail", S_IRUGO|S_IWUSR, proc_fault_inject_operations), > @@ -3391,7 +3407,7 @@ static int proc_tid_comm_permission(struct inode > *inode, int mask) #ifdef CONFIG_AUDITSYSCALL > REG("loginuid", S_IWUSR|S_IRUGO, proc_loginuid_operations), > REG("sessionid", S_IRUGO, proc_sessionid_operations), > - REG("containerid", S_IWUSR, proc_containerid_operations), > + REG("containerid", S_IWUSR|S_IRUSR, proc_containerid_operations), > #endif > #ifdef CONFIG_FAULT_INJECTION > REG("make-it-fail", S_IRUGO|S_IWUSR, proc_fault_inject_operations),
Steve Grubb <sgrubb@redhat.com> writes: > On Friday, March 16, 2018 5:00:40 AM EDT Richard Guy Briggs wrote: >> Add support for reading the container ID from the proc filesystem. > > I think this could be useful in general. Please consider this to be part of > the full patch set and not something merely used to debug the patches. Only with an audit specific name. As it is: Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com> The truth is the containerid name really stinks and is quite confusing and does not imply that the label applies only to audit. And little things like this make me extremely uncofortable with it. Eric
On Mon, May 21, 2018 at 3:19 PM, Eric W. Biederman <ebiederm@xmission.com> wrote: > Steve Grubb <sgrubb@redhat.com> writes: > >> On Friday, March 16, 2018 5:00:40 AM EDT Richard Guy Briggs wrote: >>> Add support for reading the container ID from the proc filesystem. >> >> I think this could be useful in general. Please consider this to be part of >> the full patch set and not something merely used to debug the patches. > > Only with an audit specific name. > > As it is: > > Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com> > > The truth is the containerid name really stinks and is quite confusing > and does not imply that the label applies only to audit. And little > things like this make me extremely uncofortable with it. It also makes the audit container ID (notice how I *always* call it the *audit* container ID? that is not an accident) available for userspace applications to abuse. Perhaps in the future we can look at ways to make this more available to applications, but this patch is not the answer.
On 2018-05-21 16:06, Paul Moore wrote: > On Mon, May 21, 2018 at 3:19 PM, Eric W. Biederman <ebiederm@xmission.com> wrote: > > Steve Grubb <sgrubb@redhat.com> writes: > >> On Friday, March 16, 2018 5:00:40 AM EDT Richard Guy Briggs wrote: > >>> Add support for reading the container ID from the proc filesystem. > >> > >> I think this could be useful in general. Please consider this to be part of > >> the full patch set and not something merely used to debug the patches. > > > > Only with an audit specific name. > > > > As it is: > > > > Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com> > > > > The truth is the containerid name really stinks and is quite confusing > > and does not imply that the label applies only to audit. And little > > things like this make me extremely uncofortable with it. > > It also makes the audit container ID (notice how I *always* call it > the *audit* container ID? that is not an accident) available for > userspace applications to abuse. Perhaps in the future we can look at > ways to make this more available to applications, but this patch is > not the answer. Do you have a productive suggestion? > paul moore - RGB -- Richard Guy Briggs <rgb@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
On Tue, May 22, 2018 at 1:35 PM, Richard Guy Briggs <rgb@redhat.com> wrote: > On 2018-05-21 16:06, Paul Moore wrote: >> On Mon, May 21, 2018 at 3:19 PM, Eric W. Biederman <ebiederm@xmission.com> wrote: >> > Steve Grubb <sgrubb@redhat.com> writes: >> >> On Friday, March 16, 2018 5:00:40 AM EDT Richard Guy Briggs wrote: >> >>> Add support for reading the container ID from the proc filesystem. >> >> >> >> I think this could be useful in general. Please consider this to be part of >> >> the full patch set and not something merely used to debug the patches. >> > >> > Only with an audit specific name. >> > >> > As it is: >> > >> > Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com> >> > >> > The truth is the containerid name really stinks and is quite confusing >> > and does not imply that the label applies only to audit. And little >> > things like this make me extremely uncofortable with it. >> >> It also makes the audit container ID (notice how I *always* call it >> the *audit* container ID? that is not an accident) available for >> userspace applications to abuse. Perhaps in the future we can look at >> ways to make this more available to applications, but this patch is >> not the answer. > > Do you have a productive suggestion? I haven't given it much thought beyond our discussions and until we get the basic audit container ID support in place (all the other parts of this patchset) I doubt I'll be giving it much thought.
diff --git a/fs/proc/base.c b/fs/proc/base.c index 6ce4fbe..f66d1e2 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -1300,6 +1300,21 @@ static ssize_t proc_sessionid_read(struct file * file, char __user * buf, .llseek = generic_file_llseek, }; +static ssize_t proc_containerid_read(struct file *file, char __user *buf, + size_t count, loff_t *ppos) +{ + struct inode *inode = file_inode(file); + struct task_struct *task = get_proc_task(inode); + ssize_t length; + char tmpbuf[TMPBUFLEN*2]; + + if (!task) + return -ESRCH; + length = scnprintf(tmpbuf, TMPBUFLEN*2, "%llu", audit_get_containerid(task)); + put_task_struct(task); + return simple_read_from_buffer(buf, count, ppos, tmpbuf, length); +} + static ssize_t proc_containerid_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos) { @@ -1330,6 +1345,7 @@ static ssize_t proc_containerid_write(struct file *file, const char __user *buf, } static const struct file_operations proc_containerid_operations = { + .read = proc_containerid_read, .write = proc_containerid_write, .llseek = generic_file_llseek, }; @@ -2996,7 +3012,7 @@ static int proc_pid_patch_state(struct seq_file *m, struct pid_namespace *ns, #ifdef CONFIG_AUDITSYSCALL REG("loginuid", S_IWUSR|S_IRUGO, proc_loginuid_operations), REG("sessionid", S_IRUGO, proc_sessionid_operations), - REG("containerid", S_IWUSR, proc_containerid_operations), + REG("containerid", S_IWUSR|S_IRUSR, proc_containerid_operations), #endif #ifdef CONFIG_FAULT_INJECTION REG("make-it-fail", S_IRUGO|S_IWUSR, proc_fault_inject_operations), @@ -3391,7 +3407,7 @@ static int proc_tid_comm_permission(struct inode *inode, int mask) #ifdef CONFIG_AUDITSYSCALL REG("loginuid", S_IWUSR|S_IRUGO, proc_loginuid_operations), REG("sessionid", S_IRUGO, proc_sessionid_operations), - REG("containerid", S_IWUSR, proc_containerid_operations), + REG("containerid", S_IWUSR|S_IRUSR, proc_containerid_operations), #endif #ifdef CONFIG_FAULT_INJECTION REG("make-it-fail", S_IRUGO|S_IWUSR, proc_fault_inject_operations),
Add support for reading the container ID from the proc filesystem. This is a read from the proc entry of the form /proc/PID/containerid where PID is the process ID of the task whose container ID is sought. The read expects up to a u64 value (unset: 18446744073709551615). Signed-off-by: Richard Guy Briggs <rgb@redhat.com> --- fs/proc/base.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)