* Re: iptables audit target causes kernel panic with iptables-persistent (kernel 3.2.78)
From: Paul Moore @ 2016-04-26 18:54 UTC (permalink / raw)
To: Lev Stipakov; +Cc: linux-audit, netfilter-devel
In-Reply-To: <nfnajo$4cj$1@ger.gmane.org>
On Tue, Apr 26, 2016 at 4:58 AM, Lev Stipakov <lstipakov@gmail.com> wrote:
> Hello,
>
> I see kernel panic with iptables-persistent package installed and one
> iptables rule with AUDIT target.
>
> root@debian7:~# uname -a
> Linux debian7 3.2.0-4-amd64 #1 SMP Debian 3.2.78-1 x86_64 GNU/Linux
>
> root@debian7:~# dpkg -l | grep iptables
> ii iptables 1.4.14-3.1
> ii iptables-persistent 0.5.7+deb7u1
>
> Steps to reproduce:
>
> 1) Install Debian 7 and iptables-persistent (see versions above)
> 2) Add iptables rule (must be OUTPUT chain):
>
> root@debian7:~# iptables -I OUTPUT -j AUDIT --type ACCEPT
>
> 3) Save rule:
>
> root@debian7:~# iptables-save > /etc/iptables/rules.v4
>
> 4) Reboot
>
> 5) Kernel panic (screenshot):
> https://www.dropbox.com/s/db40e5kc10e4ddg/kernel_panic2.png?dl=0
>
>
> I cannot reproduce it on (one of) previous kernel version:
>
> lev@debi7:~$ uname -a
> Linux debi7 3.2.0-4-amd64 #1 SMP Debian 3.2.73-2+deb7u2 x86_64 GNU/Linux
>
> lev@debi7:~$ dpkg -l | grep iptables
> ii iptables 1.4.14-3.1
> ii iptables-persistent 0.5.7+deb7u1
Unfortunately I don't have a Debian system available to test, but have
you tried reproducing this on a more modern kernel?
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: iptables audit target causes kernel panic with iptables-persistent (kernel 3.2.78)
From: Lev Stipakov @ 2016-04-26 12:18 UTC (permalink / raw)
To: linux-audit
In-Reply-To: <nfnajo$4cj$1@ger.gmane.org>
Kernel crash dump:
[ 217.819774] piix4_smbus 0000:00:07.0: SMBus base address
uninitialized - upgrade BIOS or use force_addr=0xaddr
[ 218.173782] Error: Driver 'pcspkr' is already registered, aborting...
[ 229.433697] BUG: unable to handle kernel paging request at
ffff88021a2fc80b
[ 229.524189] IP: [<ffffffffa03e3330>] audit_tg+0xb9/0x15b [xt_AUDIT]
[ 229.713702] PGD 1606063 PUD 0
[ 229.714117] Oops: 0000 [#1] SMP
[ 229.714479] CPU 0
[ 229.714652] Modules linked in: xt_AUDIT parport_pc ppdev lp parport
bnep bluetooth rfkill ip6table_filter ip6_tables iptable_filter
ip_tables x_tables uinput nfsd nfs nfs_acl auth_rpcgss fscache lockd
sunrpc loop crc32c_intel aesni_intel battery ac power_supply pcspkr
processor video aes_x86_64 thermal_sys psmouse joydev evdev serio_raw
button aes_generic cryptd snd_intel8x0 snd_ac97_codec snd_pcm
snd_page_alloc snd_timer snd soundcore vboxguest(O) i2c_piix4 i2c_core
ac97_bus ext4 crc16 jbd2 mbcache usbhid hid sg sr_mod sd_mod crc_t10dif
cdrom ata_generic ata_piix ohci_hcd ehci_hcd ahci libahci libata usbcore
usb_common e1000 scsi_mod [last unloaded: scsi_wait_scan]
[ 230.154897]
[ 230.223490] Pid: 0, comm: swapper/0 Tainted: G O
3.2.0-4-amd64 #1 Debian 3.2.78-1 innotek GmbH VirtualBox/VirtualBox
[ 230.594007] RIP: 0010:[<ffffffffa03e3330>] [<ffffffffa03e3330>]
audit_tg+0xb9/0x15b [xt_AUDIT]
[ 230.963683] RSP: 0018:ffff88011fc03be0 EFLAGS: 00010286
[ 231.053744] RAX: 0000000000000000 RBX: ffff880119f8aac0 RCX:
ffff88021a2fc7ff
[ 231.433840] RDX: 000000000000005c RSI: ffffffffa03e412f RDI:
ffff88011a8beac0
[ 231.603982] RBP: ffff88011fc03ce0 R08: ffff880119e15000 R09:
00000000fffffff8
[ 231.724164] R10: 0000000000000078 R11: 0000000000000000 R12:
ffff88011a8beac0
[ 231.725226] R13: ffff8801181cb658 R14: ffff880119f8aac0 R15:
ffff8801181cb638
[ 231.744298] FS: 0000000000000000(0000) GS:ffff88011fc00000(0000)
knlGS:0000000000000000
[ 231.745494] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 231.754042] CR2: ffff88021a2fc80b CR3: 0000000119e58000 CR4:
00000000000406f0
[ 231.755131] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000
[ 231.763888] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7:
0000000000000400
[ 231.764930] Process swapper/0 (pid: 0, threadinfo ffffffff81600000,
task ffffffff8160d020)
[ 231.766108] Stack:
[ 231.772178] ffff880117e3e000 0000000000000000 0000009d00000001
ffff8801181cb5c8
[ 231.794053] ffff880119e1a540 ffff88011fc1a88c ffff88011a2fc810
ffffffffa035b0f4
[ 231.804858] 0000000000000046 ffff880117e3e000 ffff880118f17e80
ffffffff8160d020
[ 231.805980] Call Trace:
[ 231.814086] <IRQ>
[ 231.814508] [<ffffffffa035b0f4>] ? ipt_do_table+0x4d7/0x556 [ip_tables]
[ 231.815478] [<ffffffff812f4470>] ? xfrm_lookup+0x3a1/0x43a
[ 231.816293] [<ffffffff810ec003>] ? virt_to_cache+0x7/0x23
[ 231.854059] [<ffffffff812b3a49>] ? nf_iterate+0x41/0x77
[ 231.864550] [<ffffffff812bc14a>] ? __skb_dequeue+0x31/0x31
[ 231.865372] [<ffffffff812b3ae7>] ? nf_hook_slow+0x68/0x101
[ 231.866184] [<ffffffff812bc14a>] ? __skb_dequeue+0x31/0x31
[ 231.880501] [<ffffffff812bd7cb>] ? nf_hook_thresh.constprop.31+0x39/0x3e
[ 231.881538] [<ffffffff812bd7ef>] ? __ip_local_out+0x1f/0x3d
[ 231.882373] [<ffffffff812bd816>] ? ip_local_out+0x9/0x19
[ 231.883171] [<ffffffff812e2aa5>] ? igmp_ifc_timer_expire+0x1b2/0x1df
[ 231.884114] [<ffffffff810526cc>] ? run_timer_softirq+0x19a/0x261
[ 231.885010] [<ffffffff812e28f3>] ? add_grec+0x364/0x364
[ 231.885799] [<ffffffff8102b2d2>] ? kvm_clock_read+0x17/0x1a
[ 231.894392] [<ffffffff8104c4c0>] ? __do_softirq+0xd7/0x1af
[ 231.895271] [<ffffffff8106b417>] ? clockevents_program_event+0xaa/0xce
[ 231.896236] [<ffffffff813594ec>] ? call_softirq+0x1c/0x30
[ 231.897055] [<ffffffff8100faad>] ? do_softirq+0x3c/0x7b
[ 231.897857] [<ffffffff8104c742>] ? irq_exit+0x3c/0x99
[ 231.904278] [<ffffffff81024670>] ? smp_apic_timer_interrupt+0x74/0x82
[ 231.905270] [<ffffffff81357d5e>] ? apic_timer_interrupt+0x6e/0x80
[ 231.906178] <EOI>
[ 231.906543] [<ffffffff810149e7>] ? mwait_idle+0x7f/0xac
[ 232.125169] [<ffffffff810149da>] ? mwait_idle+0x72/0xac
[ 232.284049] [<ffffffff8100d24c>] ? cpu_idle+0xaf/0xf2
[ 232.284927] [<ffffffff816aab3b>] ? start_kernel+0x3bd/0x3c8
[ 232.285814] [<ffffffff816aa140>] ? early_idt_handlers+0x140/0x140
[ 232.286728] [<ffffffff816aa3c4>] ? x86_64_start_kernel+0x104/0x111
[ 232.287645] Code: 8b 43 20 48 85 c0 74 78 66 83 b8 c4 01 00 00 01 75
6e 8b 8b c8 00 00 00 31 c0 48 c7 c6 2f 41 3e a0 48 03 8b d8 00 00 00 4c
89 e7 <66> 44 8b 41 0c 48 8d 51 06 66 41 c1 c0 08 45 0f b7 c0 e8 cd 5e
[ 232.505392] RIP [<ffffffffa03e3330>] audit_tg+0xb9/0x15b [xt_AUDIT]
[ 232.506338] RSP <ffff88011fc03be0>
[ 232.524441] CR2: ffff88021a2fc80b
[ 232.534296] ---[ end trace 3c9efffc5c9e0cae ]---
[ 232.535051] Kernel panic - not syncing: Fatal exception in interrupt
[ 232.535973] Pid: 0, comm: swapper/0 Tainted: G D O
3.2.0-4-amd64 #1 Debian 3.2.78-1
[ 232.537158] Call Trace:
[ 232.537543] <IRQ> [<ffffffff8134b6b7>] ? panic+0x95/0x1a2
[ 232.538388] [<ffffffff81352247>] ? _raw_spin_unlock_irqrestore+0xe/0xf
[ 232.539358] [<ffffffff8135310c>] ? oops_end+0xa9/0xb6
[ 232.540123] [<ffffffff8134afd6>] ? no_context+0x1ff/0x20e
[ 232.540968] [<ffffffff8134a8ed>] ? pud_offset+0x16/0x35
[ 232.564725] [<ffffffff81355109>] ? do_page_fault+0x1b6/0x345
[ 2232.604314] [<ffffffff81089205>] ? audit_log_vformat+0xcb/0xda
[ 232.914225] [<ffffffff811b4025>] ? vsnprintf+0x3ee/0x427
[ 233.014428] [<ffffffff81089257>] ? audit_log_format+0x43/0x48
[ 233.164204] [<ffffffff81352815>] ? page_fault+0x25/0x30
[ 233.374338] [<ffffffffa03e3330>] ? audit_tg+0xb9/0x15b [xt_AUDIT]
[ 233.405031] [<ffffffffa035b0f4>] ? ipt_do_table+0x4d7/0x556 [ip_tablss]
[ 233.924368] [<ffffffff812f4470>] ? xfrm_lookup+0x3a1/0x43a
[ 234.214539] [<ffffffff810ec003>] ? virt_to_cache+0x7/0x23
[ 2234.274907] [<ffffffff812b3a49>] ? nf_iterate+0x41/0x77
[ 234.342667] [<ffffffff812bc14a>] ? __skb_dequeue+0x31/0x31
[ 234.495100] [<ffffffff812b3ae7>] ? nf_hook_slow+0x68/0x101
[ 234.535275] [<ffffffff812bc14a>] ? __skb_dequeue+0x31/0x31
[ 234.614601] [<ffffffff812bd7cb>] ? nf_hook_thresh.constprop.31+0x39/0x3e
[ 234.714592] [<ffffffff812bd7ef>] ? __ip_local_out+0x1f/0x3
[ 2234.836013] [<ffffffff812bd816>] ? ip_local_out+0x9/0x19
[ 2234.925049] [<ffffffff812e2aa5>] ? igmp_ifc_timer_expire+0x1b2/0x1df
[ 235.014937] [<ffffffff810526cc>] ? run_timer_softirq+0x19a/0x261
[ 235.083763] [<ffffffff812e28f3>] ? add_grec+0x364/0x364
[ 235.314747] [<ffffffff8102b2d2>] ? kvm_clock_read+0x17/0x1a
[ 235.380032] [<ffffffff8104c4c0>] ? __do_softirq+0xd7/0x1af
[ 235.495023] [<ffffffff8106b417>] ? clockevents_program_event+0xaa/0xce
[ 2235.575418] [<ffffffff813594ec>] ? call_softirq+0x1c/0x30
[ 255.725267] [<ffffffff8100faad>] ? do_softirq+0x3c/0x7b
[ 235.914972] [<ffffffff8104c742>] ? irq_exit+0x3c/0x99
[ 235.995091] [<ffffffff81024670>] ? smp_apic_timer_interrupt+0x74/0x82
[ 236.035736] [<ffffffff81357d5e>] ? apic_timer_interrupt+0x6e/0x80
[ 236.104947] <EOI> [<ffffffff810149e7>] ? mwait_idle+0x7f/0xac
[ 236.254760] [<ffffffff810149da>] ? mwait_idle+0x72/0xac
[ 236.358975] [<ffffffff8100d24c>] ? cpu_idle+0xaf/0x22
[ 236.463513] [<ffffffff816aab3b>] ? start_kernel+0x3bd/0x3c8
[ 236.515132] [<ffffffff816aa140>] ? early_idt_handlers+0x140/0x140
[ 2236.536116] [<ffffffff816aa3c4>] ? x86_64_start_kernel+0x104/0x111
On 26.04.2016 11:58, Lev Stipakov wrote:
> Hello,
>
> I see kernel panic with iptables-persistent package installed and one
> iptables rule with AUDIT target.
>
> root@debian7:~# uname -a
> Linux debian7 3.2.0-4-amd64 #1 SMP Debian 3.2.78-1 x86_64 GNU/Linux
>
> root@debian7:~# dpkg -l | grep iptables
> ii iptables 1.4.14-3.1
> ii iptables-persistent 0.5.7+deb7u1
>
> Steps to reproduce:
>
> 1) Install Debian 7 and iptables-persistent (see versions above)
> 2) Add iptables rule (must be OUTPUT chain):
>
> root@debian7:~# iptables -I OUTPUT -j AUDIT --type ACCEPT
>
> 3) Save rule:
>
> root@debian7:~# iptables-save > /etc/iptables/rules.v4
>
> 4) Reboot
>
> 5) Kernel panic (screenshot):
> https://www.dropbox.com/s/db40e5kc10e4ddg/kernel_panic2.png?dl=0
>
>
> I cannot reproduce it on (one of) previous kernel version:
>
> lev@debi7:~$ uname -a
> Linux debi7 3.2.0-4-amd64 #1 SMP Debian 3.2.73-2+deb7u2 x86_64 GNU/Linux
>
> lev@debi7:~$ dpkg -l | grep iptables
> ii iptables 1.4.14-3.1
> ii iptables-persistent 0.5.7+deb7u1
>
>
> -Lev
>
>
^ permalink raw reply
* iptables audit target causes kernel panic with iptables-persistent (kernel 3.2.78)
From: Lev Stipakov @ 2016-04-26 8:58 UTC (permalink / raw)
To: linux-audit; +Cc: netfilter-devel
Hello,
I see kernel panic with iptables-persistent package installed and one
iptables rule with AUDIT target.
root@debian7:~# uname -a
Linux debian7 3.2.0-4-amd64 #1 SMP Debian 3.2.78-1 x86_64 GNU/Linux
root@debian7:~# dpkg -l | grep iptables
ii iptables 1.4.14-3.1
ii iptables-persistent 0.5.7+deb7u1
Steps to reproduce:
1) Install Debian 7 and iptables-persistent (see versions above)
2) Add iptables rule (must be OUTPUT chain):
root@debian7:~# iptables -I OUTPUT -j AUDIT --type ACCEPT
3) Save rule:
root@debian7:~# iptables-save > /etc/iptables/rules.v4
4) Reboot
5) Kernel panic (screenshot):
https://www.dropbox.com/s/db40e5kc10e4ddg/kernel_panic2.png?dl=0
I cannot reproduce it on (one of) previous kernel version:
lev@debi7:~$ uname -a
Linux debi7 3.2.0-4-amd64 #1 SMP Debian 3.2.73-2+deb7u2 x86_64 GNU/Linux
lev@debi7:~$ dpkg -l | grep iptables
ii iptables 1.4.14-3.1
ii iptables-persistent 0.5.7+deb7u1
-Lev
^ permalink raw reply
* Re: New field to auditd.conf file
From: Richard Guy Briggs @ 2016-04-26 0:37 UTC (permalink / raw)
To: Deepika Sundar; +Cc: linux-audit
In-Reply-To: <CAHj_pNf8nnRrwpE+_QLzq0Ef5gUym33Vy3-j+u3jL+knf1xqXw@mail.gmail.com>
On 16/04/25, Deepika Sundar wrote:
> I wanted to add the namespace information in the audit record for example
> pid_ns,user_ns,net_ns ,Is there any possibility to add this field inside
> Audit structure?
We've been looking at this issue for several years now and don't have an
obvious solution yet. There has been discussion on this list. It is on
the radar:
https://bugzilla.redhat.com/show_bug.cgi?id=1045666
> On Thu, Apr 21, 2016 at 6:28 PM, Paul Moore <pmoore@redhat.com> wrote:
> > As we've already mentioned several times, we can make no guarantees
> > regarding functionality or compatibility without seeing your code.
> > While it may be frustrating, this is how Open Source development
> > works.
> >
> > If you are interested in our help you will need to describe, in
> > detail, what you are trying to do and ideally post your existing code
> > so it can be reviewed.
> >
> > On Thu, Apr 21, 2016 at 1:25 AM, Deepika Sundar
> > <sundar.deepika18@gmail.com> wrote:
> > > Okay,If I update the Ausearch/aureport in order to aware of the new
> > field in
> > > the audit log structure can it be feasible one?
> > >
> > > On Wed, Apr 20, 2016 at 6:00 PM, Steve Grubb <sgrubb@redhat.com> wrote:
> > >>
> > >> On Wednesday, April 20, 2016 10:05:42 AM Deepika Sundar wrote:
> > >> > In general way,Is there any compatibility issues if audit log
> > structure
> > >> > gets modified?
> > >>
> > >> Yes, there can be problems if the log structure gets modified.
> > >> Ausearch/report
> > >> are highly optimized for an exact format.
> > >>
> > >> -Steve
> > >>
> > >>
> > >> > On Wed, Apr 13, 2016 at 6:01 PM, Steve Grubb <sgrubb@redhat.com>
> > wrote:
> > >> > > On Wednesday, April 13, 2016 11:03:43 AM Deepika Sundar wrote:
> > >> > > > As per my understanding audit log structure can be extendible
> > based
> > >> > > > on
> > >> > > > requirements and in my project I need to add the identifier field
> > >> > > > for
> > >> > > > the
> > >> > > > application and as of now I couldn't able to revel the What
> > >> > > > application
> > >> > > > trying to develop to update.So,Is there any possibility that
> > without
> > >> > > > breaking any Compatibility issues I can do it ?
> > >> > >
> > >> > > I have no idea what you are doing so there is no guarantee that it
> > >> > > won't
> > >> > > break
> > >> > > something. If your project is going to be released as open source
> > its
> > >> > > generally best to collaborate with people so that problems can be
> > >> > > pointed
> > >> > > out.
> > >> > > Otherwise you risk spending a lot of time on something only to have
> > it
> > >> > > rejected.
> > >> > >
> > >> > > -Steve
> > >> > >
> > >> > > > OR If any compatibility issues please specify .
> > >> > > >
> > >> > > > On Fri, Apr 8, 2016 at 12:12 AM, Paul Moore <paul@paul-moore.com>
> > >> > > > wrote:
> > >> > > > > On Thu, Apr 7, 2016 at 12:47 AM, Deepika Sundar
> > >> > > > >
> > >> > > > > <sundar.deepika18@gmail.com> wrote:
> > >> > > > > > In the same way, in the kernel side
> > >> > > > > > Can I able to add one new field to the audit log structure
> > >> > > > > > without
> > >> > > > >
> > >> > > > > breaking
> > >> > > > >
> > >> > > > > > Compatibility? If so,
> > >> > > > > >
> > >> > > > > > 1.How can I add new field without breaking compatibility?
> > >> > > > > >
> > >> > > > > > or
> > >> > > > > >
> > >> > > > > > 2.Is there any reserve field in audit log structure so that
> > I
> > >> > > > > > can
> > >> > >
> > >> > > make
> > >> > >
> > >> > > > > use
> > >> > > > >
> > >> > > > > > of it?
> > >> > > > >
> > >> > > > > You need to be more specific about what you are trying to do.
> > >> > > > > Speaking generally, unless you work to get your changed merged
> > >> > > > > into
> > >> > > > > the upstream kernel and userspace tools we cannot guarantee
> > >> > > > > present or
> > >> > > > > future compatibility.
> > >> > > > >
> > >> > > > > --
> > >> > > > > paul moore
> > >> > > > > www.paul-moore.com
> > >>
> > >
> > >
> > > --
> > > Linux-audit mailing list
> > > Linux-audit@redhat.com
> > > https://www.redhat.com/mailman/listinfo/linux-audit
> >
> >
> >
> > --
> > paul moore
> > security @ redhat
> >
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
- RGB
--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635
^ permalink raw reply
* Re: New field to auditd.conf file
From: Deepika Sundar @ 2016-04-25 6:56 UTC (permalink / raw)
To: Paul Moore; +Cc: linux-audit
In-Reply-To: <CAGH-KguWLTY=YCq-jVmh5UmiiCXq4LWrK29mt3tjtp97OhRp1A@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 3722 bytes --]
I wanted to add the namespace information in the audit record for example
pid_ns,user_ns,net_ns ,Is there any possibility to add this field inside
Audit structure?
On Thu, Apr 21, 2016 at 6:28 PM, Paul Moore <pmoore@redhat.com> wrote:
> As we've already mentioned several times, we can make no guarantees
> regarding functionality or compatibility without seeing your code.
> While it may be frustrating, this is how Open Source development
> works.
>
> If you are interested in our help you will need to describe, in
> detail, what you are trying to do and ideally post your existing code
> so it can be reviewed.
>
> On Thu, Apr 21, 2016 at 1:25 AM, Deepika Sundar
> <sundar.deepika18@gmail.com> wrote:
> > Okay,If I update the Ausearch/aureport in order to aware of the new
> field in
> > the audit log structure can it be feasible one?
> >
> > On Wed, Apr 20, 2016 at 6:00 PM, Steve Grubb <sgrubb@redhat.com> wrote:
> >>
> >> On Wednesday, April 20, 2016 10:05:42 AM Deepika Sundar wrote:
> >> > In general way,Is there any compatibility issues if audit log
> structure
> >> > gets modified?
> >>
> >> Yes, there can be problems if the log structure gets modified.
> >> Ausearch/report
> >> are highly optimized for an exact format.
> >>
> >> -Steve
> >>
> >>
> >> > On Wed, Apr 13, 2016 at 6:01 PM, Steve Grubb <sgrubb@redhat.com>
> wrote:
> >> > > On Wednesday, April 13, 2016 11:03:43 AM Deepika Sundar wrote:
> >> > > > As per my understanding audit log structure can be extendible
> based
> >> > > > on
> >> > > > requirements and in my project I need to add the identifier field
> >> > > > for
> >> > > > the
> >> > > > application and as of now I couldn't able to revel the What
> >> > > > application
> >> > > > trying to develop to update.So,Is there any possibility that
> without
> >> > > > breaking any Compatibility issues I can do it ?
> >> > >
> >> > > I have no idea what you are doing so there is no guarantee that it
> >> > > won't
> >> > > break
> >> > > something. If your project is going to be released as open source
> its
> >> > > generally best to collaborate with people so that problems can be
> >> > > pointed
> >> > > out.
> >> > > Otherwise you risk spending a lot of time on something only to have
> it
> >> > > rejected.
> >> > >
> >> > > -Steve
> >> > >
> >> > > > OR If any compatibility issues please specify .
> >> > > >
> >> > > > On Fri, Apr 8, 2016 at 12:12 AM, Paul Moore <paul@paul-moore.com>
> >> > > > wrote:
> >> > > > > On Thu, Apr 7, 2016 at 12:47 AM, Deepika Sundar
> >> > > > >
> >> > > > > <sundar.deepika18@gmail.com> wrote:
> >> > > > > > In the same way, in the kernel side
> >> > > > > > Can I able to add one new field to the audit log structure
> >> > > > > > without
> >> > > > >
> >> > > > > breaking
> >> > > > >
> >> > > > > > Compatibility? If so,
> >> > > > > >
> >> > > > > > 1.How can I add new field without breaking compatibility?
> >> > > > > >
> >> > > > > > or
> >> > > > > >
> >> > > > > > 2.Is there any reserve field in audit log structure so that
> I
> >> > > > > > can
> >> > >
> >> > > make
> >> > >
> >> > > > > use
> >> > > > >
> >> > > > > > of it?
> >> > > > >
> >> > > > > You need to be more specific about what you are trying to do.
> >> > > > > Speaking generally, unless you work to get your changed merged
> >> > > > > into
> >> > > > > the upstream kernel and userspace tools we cannot guarantee
> >> > > > > present or
> >> > > > > future compatibility.
> >> > > > >
> >> > > > > --
> >> > > > > paul moore
> >> > > > > www.paul-moore.com
> >>
> >
> >
> > --
> > Linux-audit mailing list
> > Linux-audit@redhat.com
> > https://www.redhat.com/mailman/listinfo/linux-audit
>
>
>
> --
> paul moore
> security @ redhat
>
[-- Attachment #1.2: Type: text/html, Size: 6011 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: PID's Mapping
From: Deepika Sundar @ 2016-04-25 6:54 UTC (permalink / raw)
To: Steve Grubb; +Cc: linux-audit
In-Reply-To: <1748089.XZrfDjoJO0@x2>
[-- Attachment #1.1: Type: text/plain, Size: 1965 bytes --]
Yeah.
When the PID's which are in the namespace application has different PID
compared to Global PID.There would be some means to map the PID's in the
kernel level.Can anyone suggest How it can be mapped?
On Wed, Apr 20, 2016 at 6:03 PM, Steve Grubb <sgrubb@redhat.com> wrote:
> On Wednesday, April 20, 2016 10:06:38 AM Deepika Sundar wrote:
> > Is there any way that can be suggested as to map PID's of namespace in
> > global?
>
> This is on the TODO list. We have been kicking around several ideas but
> have
> not come to a conclusion about what exactly needs to be done. The upshot of
> this is that basically containers have no support.
>
> -Steve
>
>
> > On Mon, Apr 18, 2016 at 8:47 PM, Paul Moore <paul@paul-moore.com> wrote:
> > > Please ask your question on the mailing list so that everyone can
> benefit.
> > >
> > > On Mon, Apr 18, 2016 at 1:34 AM, Deepika Sundar
> > >
> > > <sundar.deepika18@gmail.com> wrote:
> > > > How it can be achieved ,Can I get any idea on this?
> > > >
> > > > On Fri, Apr 15, 2016 at 4:12 AM, Paul Moore <paul@paul-moore.com>
> wrote:
> > > >> On Wed, Apr 13, 2016 at 1:43 AM, sowndarya kumar
> > > >>
> > > >> <sowndarya.nadar@gmail.com> wrote:
> > > >> > Hi
> > > >> >
> > > >> > Is there any way to map the PID's seen in the namespace
> application
> > >
> > > with
> > >
> > > >> > the
> > > >> > PID's seen in global?
> > > >> > If it can be done please provide the documentation or idea on how
> it
> > >
> > > can
> > >
> > > >> > be
> > > >> > done.
> > > >>
> > > >> In general the audit subsystem doesn't pay attention to namespaces,
> > > >> all PIDs reported to userspace are reported with respect to the init
> > > >> namespace.
> > > >>
> > > >> --
> > > >> paul moore
> > > >> www.paul-moore.com
> > > >>
> > > >> --
> > > >> Linux-audit mailing list
> > > >> Linux-audit@redhat.com
> > > >> https://www.redhat.com/mailman/listinfo/linux-audit
> > >
> > > --
> > > paul moore
> > > www.paul-moore.com
>
>
[-- Attachment #1.2: Type: text/html, Size: 3568 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Fwd: PID's Mapping
From: Deepika Sundar @ 2016-04-25 6:53 UTC (permalink / raw)
To: linux-audit
In-Reply-To: <CAHj_pNdAoTjNw_R3oxWGaEH+xBmkY8SDJK710V3HY9Om4EYfgQ@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 2156 bytes --]
---------- Forwarded message ----------
From: Deepika Sundar <sundar.deepika18@gmail.com>
Date: Mon, Apr 25, 2016 at 12:22 PM
Subject: Re: PID's Mapping
To: Steve Grubb <sgrubb@redhat.com>
Yeah.
When the PID's which are in the namespace application has different PID
compared to Global PID.There would be some means to map the PID's in the
kernel level.Can anyone suggest How it can be mapped?
On Wed, Apr 20, 2016 at 6:03 PM, Steve Grubb <sgrubb@redhat.com> wrote:
> On Wednesday, April 20, 2016 10:06:38 AM Deepika Sundar wrote:
> > Is there any way that can be suggested as to map PID's of namespace in
> > global?
>
> This is on the TODO list. We have been kicking around several ideas but
> have
> not come to a conclusion about what exactly needs to be done. The upshot of
> this is that basically containers have no support.
>
> -Steve
>
>
> > On Mon, Apr 18, 2016 at 8:47 PM, Paul Moore <paul@paul-moore.com> wrote:
> > > Please ask your question on the mailing list so that everyone can
> benefit.
> > >
> > > On Mon, Apr 18, 2016 at 1:34 AM, Deepika Sundar
> > >
> > > <sundar.deepika18@gmail.com> wrote:
> > > > How it can be achieved ,Can I get any idea on this?
> > > >
> > > > On Fri, Apr 15, 2016 at 4:12 AM, Paul Moore <paul@paul-moore.com>
> wrote:
> > > >> On Wed, Apr 13, 2016 at 1:43 AM, sowndarya kumar
> > > >>
> > > >> <sowndarya.nadar@gmail.com> wrote:
> > > >> > Hi
> > > >> >
> > > >> > Is there any way to map the PID's seen in the namespace
> application
> > >
> > > with
> > >
> > > >> > the
> > > >> > PID's seen in global?
> > > >> > If it can be done please provide the documentation or idea on how
> it
> > >
> > > can
> > >
> > > >> > be
> > > >> > done.
> > > >>
> > > >> In general the audit subsystem doesn't pay attention to namespaces,
> > > >> all PIDs reported to userspace are reported with respect to the init
> > > >> namespace.
> > > >>
> > > >> --
> > > >> paul moore
> > > >> www.paul-moore.com
> > > >>
> > > >> --
> > > >> Linux-audit mailing list
> > > >> Linux-audit@redhat.com
> > > >> https://www.redhat.com/mailman/listinfo/linux-audit
> > >
> > > --
> > > paul moore
> > > www.paul-moore.com
>
>
[-- Attachment #1.2: Type: text/html, Size: 4015 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [PATCH V4] audit: add tty field to LOGIN event
From: Peter Hurley @ 2016-04-22 17:16 UTC (permalink / raw)
To: Richard Guy Briggs, linux-audit, pmoore; +Cc: linux-kernel, sgrubb, eparis
In-Reply-To: <dedf98fab21b8bd8bc96a1200ba1c3c1a105324f.1461260693.git.rgb@redhat.com>
On 04/21/2016 11:14 AM, Richard Guy Briggs wrote:
> The tty field was missing from AUDIT_LOGIN events.
>
> Refactor code to create a new function audit_get_tty(), using it to
> replace the call in audit_log_task_info() and to add it to
> audit_log_set_loginuid(). Lock and bump the kref to protect it, adding
> audit_put_tty() alias to decrement it.
>
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>
> V4: Add missing prototype for audit_put_tty() when audit syscall is not
> enabled (MIPS).
>
> V3: Introduce audit_put_tty() alias to decrement kref.
>
> V2: Use kref to protect tty signal struct while in use.
>
> ---
>
> include/linux/audit.h | 24 ++++++++++++++++++++++++
> kernel/audit.c | 18 +++++-------------
> kernel/auditsc.c | 8 ++++++--
> 3 files changed, 35 insertions(+), 15 deletions(-)
>
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index b40ed5d..32cdafb 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -26,6 +26,7 @@
> #include <linux/sched.h>
> #include <linux/ptrace.h>
> #include <uapi/linux/audit.h>
> +#include <linux/tty.h>
>
> #define AUDIT_INO_UNSET ((unsigned long)-1)
> #define AUDIT_DEV_UNSET ((dev_t)-1)
> @@ -343,6 +344,23 @@ static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
> return tsk->sessionid;
> }
>
> +static inline struct tty_struct *audit_get_tty(struct task_struct *tsk)
> +{
> + struct tty_struct *tty = NULL;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&tsk->sighand->siglock, flags);
> + if (tsk->signal)
> + tty = tty_kref_get(tsk->signal->tty);
> + spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
Not that I'm objecting because I get that you're just refactoring
existing code, but I thought I'd point out some stuff.
1. There's no need to check if signal_struct is NULL (ie. tsk->signal)
because if it is, this will blow up trying to dereference the
sighand_struct (ie tsk->sighand).
2. The existing usage is always tsk==current
3. If the idea is to make this invulnerable to tsk being gone, then
the usage is unsafe anyway.
So ultimately (but not necessarily for this patch) I'd prefer that either
a. audit use existing tty api instead of open-coding, or
b. add any tty api functions required.
Regards,
Peter Hurley
> + return tty;
> +}
> +
> +static inline void audit_put_tty(struct tty_struct *tty)
> +{
> + tty_kref_put(tty);
> +}
> +
> extern void __audit_ipc_obj(struct kern_ipc_perm *ipcp);
> extern void __audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid, umode_t mode);
> extern void __audit_bprm(struct linux_binprm *bprm);
> @@ -500,6 +518,12 @@ static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
> {
> return -1;
> }
> +static inline struct tty_struct *audit_get_tty(struct task_struct *tsk)
> +{
> + return NULL;
> +}
> +static inline void audit_put_tty(struct tty_struct *tty)
> +{ }
> static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
> { }
> static inline void audit_ipc_set_perm(unsigned long qbytes, uid_t uid,
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 3a3e5de..7edd776 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -64,7 +64,6 @@
> #include <linux/security.h>
> #endif
> #include <linux/freezer.h>
> -#include <linux/tty.h>
> #include <linux/pid_namespace.h>
> #include <net/netns/generic.h>
>
> @@ -1873,21 +1872,14 @@ void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk)
> {
> const struct cred *cred;
> char comm[sizeof(tsk->comm)];
> - char *tty;
> + struct tty_struct *tty;
>
> if (!ab)
> return;
>
> /* tsk == current */
> cred = current_cred();
> -
> - spin_lock_irq(&tsk->sighand->siglock);
> - if (tsk->signal && tsk->signal->tty && tsk->signal->tty->name)
> - tty = tsk->signal->tty->name;
> - else
> - tty = "(none)";
> - spin_unlock_irq(&tsk->sighand->siglock);
> -
> + tty = audit_get_tty(tsk);
> audit_log_format(ab,
> " ppid=%d pid=%d auid=%u uid=%u gid=%u"
> " euid=%u suid=%u fsuid=%u"
> @@ -1903,11 +1895,11 @@ void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk)
> from_kgid(&init_user_ns, cred->egid),
> from_kgid(&init_user_ns, cred->sgid),
> from_kgid(&init_user_ns, cred->fsgid),
> - tty, audit_get_sessionid(tsk));
> -
> + tty ? tty_name(tty) : "(none)",
> + audit_get_sessionid(tsk));
> + audit_put_tty(tty);
> audit_log_format(ab, " comm=");
> audit_log_untrustedstring(ab, get_task_comm(comm, tsk));
> -
> audit_log_d_path_exe(ab, tsk->mm);
> audit_log_task_context(ab);
> }
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 195ffae..71e14d8 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1980,6 +1980,7 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
> {
> struct audit_buffer *ab;
> uid_t uid, oldloginuid, loginuid;
> + struct tty_struct *tty;
>
> if (!audit_enabled)
> return;
> @@ -1987,14 +1988,17 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
> uid = from_kuid(&init_user_ns, task_uid(current));
> oldloginuid = from_kuid(&init_user_ns, koldloginuid);
> loginuid = from_kuid(&init_user_ns, kloginuid),
> + tty = audit_get_tty(current);
>
> ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_LOGIN);
> if (!ab)
> return;
> audit_log_format(ab, "pid=%d uid=%u", task_pid_nr(current), uid);
> audit_log_task_context(ab);
> - audit_log_format(ab, " old-auid=%u auid=%u old-ses=%u ses=%u res=%d",
> - oldloginuid, loginuid, oldsessionid, sessionid, !rc);
> + audit_log_format(ab, " old-auid=%u auid=%u tty=%s old-ses=%u ses=%u res=%d",
> + oldloginuid, loginuid, tty ? tty_name(tty) : "(none)",
> + oldsessionid, sessionid, !rc);
> + audit_put_tty(tty);
> audit_log_end(ab);
> }
>
>
^ permalink raw reply
* Re: [PATCH V4] audit: add tty field to LOGIN event
From: Paul Moore @ 2016-04-22 15:02 UTC (permalink / raw)
To: Richard Guy Briggs; +Cc: linux-audit, linux-kernel, peter
In-Reply-To: <20160422035030.GL4577@madcap2.tricolour.ca>
On Thu, Apr 21, 2016 at 11:50 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> On 16/04/21, Paul Moore wrote:
>> On Thu, Apr 21, 2016 at 2:14 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
>> > The tty field was missing from AUDIT_LOGIN events.
>> >
>> > Refactor code to create a new function audit_get_tty(), using it to
>> > replace the call in audit_log_task_info() and to add it to
>> > audit_log_set_loginuid(). Lock and bump the kref to protect it, adding
>> > audit_put_tty() alias to decrement it.
>> >
>> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
>> > ---
>> >
>> > V4: Add missing prototype for audit_put_tty() when audit syscall is not
>> > enabled (MIPS).
>> >
>> > V3: Introduce audit_put_tty() alias to decrement kref.
>> >
>> > V2: Use kref to protect tty signal struct while in use.
>> >
>> > ---
>> >
>> > include/linux/audit.h | 24 ++++++++++++++++++++++++
>> > kernel/audit.c | 18 +++++-------------
>> > kernel/auditsc.c | 8 ++++++--
>> > 3 files changed, 35 insertions(+), 15 deletions(-)
>> >
>> > diff --git a/include/linux/audit.h b/include/linux/audit.h
>> > index b40ed5d..32cdafb 100644
>> > --- a/include/linux/audit.h
>> > +++ b/include/linux/audit.h
>> > @@ -26,6 +26,7 @@
>> > #include <linux/sched.h>
>> > #include <linux/ptrace.h>
>> > #include <uapi/linux/audit.h>
>> > +#include <linux/tty.h>
>> >
>> > #define AUDIT_INO_UNSET ((unsigned long)-1)
>> > #define AUDIT_DEV_UNSET ((dev_t)-1)
>> > @@ -343,6 +344,23 @@ static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
>> > return tsk->sessionid;
>> > }
>> >
>> > +static inline struct tty_struct *audit_get_tty(struct task_struct *tsk)
>> > +{
>> > + struct tty_struct *tty = NULL;
>> > + unsigned long flags;
>> > +
>> > + spin_lock_irqsave(&tsk->sighand->siglock, flags);
>> > + if (tsk->signal)
>> > + tty = tty_kref_get(tsk->signal->tty);
>> > + spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
>> > + return tty;
>> > +}
>> > +
>> > +static inline void audit_put_tty(struct tty_struct *tty)
>> > +{
>> > + tty_kref_put(tty);
>> > +}
>>
>> I'm generally not a big fan of defining functions as inlines in header
>> files, with the general exception of dummy functions that will get
>> compiled out. Although I guess there might be some performance
>> argument to be made wrt audit_log_task_info().
>
> I did reflect on that initially and decided this was the least
> disruptive way to implement it. There are others similar around it and
> when I started it wasn't as involved, but now it is starting to push the
> limits with the kref bits...
Yeah, that is why I mentioned it, it is sort of borderline in my
opinion of what I would consider acceptable in a header file, barring
some special case.
>> I guess I'm fine with this, but what was the idea behind the static
>> inlines in audit.h? Performance, or something else?
>
> Can't say I remember... I was tempted to put it in as a define, but
> decided to stick with the existing style, right? :-)
No, definitely not a cpp macro, we'd lose type checking and other
stuff. My debate is whether or not this should life fully in the
header file vs simply a prototype in the header and the definition in
a C file. This is the first function where we are actually putting
content into linux/audit.h, all of the existing function definitions
there are dummy functions or simple wrappers for performance reasons.
>> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
>> > index 195ffae..71e14d8 100644
>> > --- a/kernel/auditsc.c
>> > +++ b/kernel/auditsc.c
>> > @@ -1980,6 +1980,7 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
>> > {
>> > struct audit_buffer *ab;
>> > uid_t uid, oldloginuid, loginuid;
>> > + struct tty_struct *tty;
>> >
>> > if (!audit_enabled)
>> > return;
>> > @@ -1987,14 +1988,17 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
>> > uid = from_kuid(&init_user_ns, task_uid(current));
>> > oldloginuid = from_kuid(&init_user_ns, koldloginuid);
>> > loginuid = from_kuid(&init_user_ns, kloginuid),
>> > + tty = audit_get_tty(current);
>> >
>> > ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_LOGIN);
>> > if (!ab)
>> > return;
>> > audit_log_format(ab, "pid=%d uid=%u", task_pid_nr(current), uid);
>> > audit_log_task_context(ab);
>> > - audit_log_format(ab, " old-auid=%u auid=%u old-ses=%u ses=%u res=%d",
>> > - oldloginuid, loginuid, oldsessionid, sessionid, !rc);
>> > + audit_log_format(ab, " old-auid=%u auid=%u tty=%s old-ses=%u ses=%u res=%d",
>> > + oldloginuid, loginuid, tty ? tty_name(tty) : "(none)",
>> > + oldsessionid, sessionid, !rc);
>> > + audit_put_tty(tty);
>> > audit_log_end(ab);
>> > }
>>
>> Because we are still using the crappy fixed string format for
>> kernel<->userspace communication, this patch will likely "break the
>> world" ... let's check with Steve but I believe the way to handle this
>> is to add the tty information to the end of the record.
>
> Well, I did try to put that keyword consistently with where it was
> inserted in other messages. My understanding was that adding an extra
> in the middle wouldn't cause a problem because the keyword scanner
> looked ahead until it found the keyword it sought. This way, older
> scanners will just hop over this keyword it wasn't seeking.
I understand the idea behind consistency, and as long as it doesn't
break the userspace parser(s) I agree with you. The good news is that
Steve says we are in the clear wrt the new field.
I'm traveling right now and I'm not sure if I'll have any time in
front of a proper computer/shell to get this merged before early next
week, but v4 looks reasonable to me. If you don't see this in the
audit#next tree by .... let's say end of day next Tuesday ... feel
free to send me a nudge.
Thanks.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH V4] audit: add tty field to LOGIN event
From: Steve Grubb @ 2016-04-22 13:13 UTC (permalink / raw)
To: linux-audit; +Cc: Paul Moore, Richard Guy Briggs, linux-kernel, peter
In-Reply-To: <CAHC9VhToUtpq94C76Xa51fKZsz-S0JUiZd-kXV2Q_705xYsgQQ@mail.gmail.com>
On Thursday, April 21, 2016 09:29:57 PM Paul Moore wrote:
> On Thu, Apr 21, 2016 at 2:14 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > The tty field was missing from AUDIT_LOGIN events.
> >
> > Refactor code to create a new function audit_get_tty(), using it to
> > replace the call in audit_log_task_info() and to add it to
> > audit_log_set_loginuid(). Lock and bump the kref to protect it, adding
> > audit_put_tty() alias to decrement it.
> >
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> >
> > V4: Add missing prototype for audit_put_tty() when audit syscall is not
> >
> > enabled (MIPS).
> >
> > V3: Introduce audit_put_tty() alias to decrement kref.
> >
> > V2: Use kref to protect tty signal struct while in use.
> >
> > ---
> >
> > include/linux/audit.h | 24 ++++++++++++++++++++++++
> > kernel/audit.c | 18 +++++-------------
> > kernel/auditsc.c | 8 ++++++--
> > 3 files changed, 35 insertions(+), 15 deletions(-)
> >
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index b40ed5d..32cdafb 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -26,6 +26,7 @@
> >
> > #include <linux/sched.h>
> > #include <linux/ptrace.h>
> > #include <uapi/linux/audit.h>
> >
> > +#include <linux/tty.h>
> >
> > #define AUDIT_INO_UNSET ((unsigned long)-1)
> > #define AUDIT_DEV_UNSET ((dev_t)-1)
> >
> > @@ -343,6 +344,23 @@ static inline unsigned int audit_get_sessionid(struct
> > task_struct *tsk)>
> > return tsk->sessionid;
> >
> > }
> >
> > +static inline struct tty_struct *audit_get_tty(struct task_struct *tsk)
> > +{
> > + struct tty_struct *tty = NULL;
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&tsk->sighand->siglock, flags);
> > + if (tsk->signal)
> > + tty = tty_kref_get(tsk->signal->tty);
> > + spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
> > + return tty;
> > +}
> > +
> > +static inline void audit_put_tty(struct tty_struct *tty)
> > +{
> > + tty_kref_put(tty);
> > +}
>
> I'm generally not a big fan of defining functions as inlines in header
> files, with the general exception of dummy functions that will get
> compiled out. Although I guess there might be some performance
> argument to be made wrt audit_log_task_info().
>
> I guess I'm fine with this, but what was the idea behind the static
> inlines in audit.h? Performance, or something else?
I think that is normal to prevent multiple definitions at link time.
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index 195ffae..71e14d8 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -1980,6 +1980,7 @@ static void audit_log_set_loginuid(kuid_t
> > koldloginuid, kuid_t kloginuid,>
> > {
> >
> > struct audit_buffer *ab;
> > uid_t uid, oldloginuid, loginuid;
> >
> > + struct tty_struct *tty;
> >
> > if (!audit_enabled)
> >
> > return;
> >
> > @@ -1987,14 +1988,17 @@ static void audit_log_set_loginuid(kuid_t
> > koldloginuid, kuid_t kloginuid,>
> > uid = from_kuid(&init_user_ns, task_uid(current));
> > oldloginuid = from_kuid(&init_user_ns, koldloginuid);
> > loginuid = from_kuid(&init_user_ns, kloginuid),
> >
> > + tty = audit_get_tty(current);
> >
> > ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_LOGIN);
> > if (!ab)
> >
> > return;
> >
> > audit_log_format(ab, "pid=%d uid=%u", task_pid_nr(current), uid);
> > audit_log_task_context(ab);
> >
> > - audit_log_format(ab, " old-auid=%u auid=%u old-ses=%u ses=%u
> > res=%d", - oldloginuid, loginuid, oldsessionid,
> > sessionid, !rc); + audit_log_format(ab, " old-auid=%u auid=%u
> > tty=%s old-ses=%u ses=%u res=%d", + oldloginuid,
> > loginuid, tty ? tty_name(tty) : "(none)", +
> > oldsessionid, sessionid, !rc);
> > + audit_put_tty(tty);
> >
> > audit_log_end(ab);
> >
> > }
>
> Because we are still using the crappy fixed string format for
> kernel<->userspace communication, this patch will likely "break the
> world" ... let's check with Steve but I believe the way to handle this
> is to add the tty information to the end of the record.
The placement is OK. Thanks for asking.
-Steve
^ permalink raw reply
* Re: [PATCH V4] audit: add tty field to LOGIN event
From: Richard Guy Briggs @ 2016-04-22 3:50 UTC (permalink / raw)
To: Paul Moore; +Cc: linux-audit, linux-kernel, peter
In-Reply-To: <CAHC9VhToUtpq94C76Xa51fKZsz-S0JUiZd-kXV2Q_705xYsgQQ@mail.gmail.com>
On 16/04/21, Paul Moore wrote:
> On Thu, Apr 21, 2016 at 2:14 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > The tty field was missing from AUDIT_LOGIN events.
> >
> > Refactor code to create a new function audit_get_tty(), using it to
> > replace the call in audit_log_task_info() and to add it to
> > audit_log_set_loginuid(). Lock and bump the kref to protect it, adding
> > audit_put_tty() alias to decrement it.
> >
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> >
> > V4: Add missing prototype for audit_put_tty() when audit syscall is not
> > enabled (MIPS).
> >
> > V3: Introduce audit_put_tty() alias to decrement kref.
> >
> > V2: Use kref to protect tty signal struct while in use.
> >
> > ---
> >
> > include/linux/audit.h | 24 ++++++++++++++++++++++++
> > kernel/audit.c | 18 +++++-------------
> > kernel/auditsc.c | 8 ++++++--
> > 3 files changed, 35 insertions(+), 15 deletions(-)
> >
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index b40ed5d..32cdafb 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -26,6 +26,7 @@
> > #include <linux/sched.h>
> > #include <linux/ptrace.h>
> > #include <uapi/linux/audit.h>
> > +#include <linux/tty.h>
> >
> > #define AUDIT_INO_UNSET ((unsigned long)-1)
> > #define AUDIT_DEV_UNSET ((dev_t)-1)
> > @@ -343,6 +344,23 @@ static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
> > return tsk->sessionid;
> > }
> >
> > +static inline struct tty_struct *audit_get_tty(struct task_struct *tsk)
> > +{
> > + struct tty_struct *tty = NULL;
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&tsk->sighand->siglock, flags);
> > + if (tsk->signal)
> > + tty = tty_kref_get(tsk->signal->tty);
> > + spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
> > + return tty;
> > +}
> > +
> > +static inline void audit_put_tty(struct tty_struct *tty)
> > +{
> > + tty_kref_put(tty);
> > +}
>
> I'm generally not a big fan of defining functions as inlines in header
> files, with the general exception of dummy functions that will get
> compiled out. Although I guess there might be some performance
> argument to be made wrt audit_log_task_info().
I did reflect on that initially and decided this was the least
disruptive way to implement it. There are others similar around it and
when I started it wasn't as involved, but now it is starting to push the
limits with the kref bits...
> I guess I'm fine with this, but what was the idea behind the static
> inlines in audit.h? Performance, or something else?
Can't say I remember... I was tempted to put it in as a define, but
decided to stick with the existing style, right? :-)
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index 195ffae..71e14d8 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -1980,6 +1980,7 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
> > {
> > struct audit_buffer *ab;
> > uid_t uid, oldloginuid, loginuid;
> > + struct tty_struct *tty;
> >
> > if (!audit_enabled)
> > return;
> > @@ -1987,14 +1988,17 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
> > uid = from_kuid(&init_user_ns, task_uid(current));
> > oldloginuid = from_kuid(&init_user_ns, koldloginuid);
> > loginuid = from_kuid(&init_user_ns, kloginuid),
> > + tty = audit_get_tty(current);
> >
> > ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_LOGIN);
> > if (!ab)
> > return;
> > audit_log_format(ab, "pid=%d uid=%u", task_pid_nr(current), uid);
> > audit_log_task_context(ab);
> > - audit_log_format(ab, " old-auid=%u auid=%u old-ses=%u ses=%u res=%d",
> > - oldloginuid, loginuid, oldsessionid, sessionid, !rc);
> > + audit_log_format(ab, " old-auid=%u auid=%u tty=%s old-ses=%u ses=%u res=%d",
> > + oldloginuid, loginuid, tty ? tty_name(tty) : "(none)",
> > + oldsessionid, sessionid, !rc);
> > + audit_put_tty(tty);
> > audit_log_end(ab);
> > }
>
> Because we are still using the crappy fixed string format for
> kernel<->userspace communication, this patch will likely "break the
> world" ... let's check with Steve but I believe the way to handle this
> is to add the tty information to the end of the record.
Well, I did try to put that keyword consistently with where it was
inserted in other messages. My understanding was that adding an extra
in the middle wouldn't cause a problem because the keyword scanner
looked ahead until it found the keyword it sought. This way, older
scanners will just hop over this keyword it wasn't seeking.
> paul moore
- RGB
^ permalink raw reply
* Re: [PATCH V4] audit: add tty field to LOGIN event
From: Paul Moore @ 2016-04-22 1:29 UTC (permalink / raw)
To: Richard Guy Briggs; +Cc: linux-audit, linux-kernel, peter
In-Reply-To: <dedf98fab21b8bd8bc96a1200ba1c3c1a105324f.1461260693.git.rgb@redhat.com>
On Thu, Apr 21, 2016 at 2:14 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> The tty field was missing from AUDIT_LOGIN events.
>
> Refactor code to create a new function audit_get_tty(), using it to
> replace the call in audit_log_task_info() and to add it to
> audit_log_set_loginuid(). Lock and bump the kref to protect it, adding
> audit_put_tty() alias to decrement it.
>
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>
> V4: Add missing prototype for audit_put_tty() when audit syscall is not
> enabled (MIPS).
>
> V3: Introduce audit_put_tty() alias to decrement kref.
>
> V2: Use kref to protect tty signal struct while in use.
>
> ---
>
> include/linux/audit.h | 24 ++++++++++++++++++++++++
> kernel/audit.c | 18 +++++-------------
> kernel/auditsc.c | 8 ++++++--
> 3 files changed, 35 insertions(+), 15 deletions(-)
>
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index b40ed5d..32cdafb 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -26,6 +26,7 @@
> #include <linux/sched.h>
> #include <linux/ptrace.h>
> #include <uapi/linux/audit.h>
> +#include <linux/tty.h>
>
> #define AUDIT_INO_UNSET ((unsigned long)-1)
> #define AUDIT_DEV_UNSET ((dev_t)-1)
> @@ -343,6 +344,23 @@ static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
> return tsk->sessionid;
> }
>
> +static inline struct tty_struct *audit_get_tty(struct task_struct *tsk)
> +{
> + struct tty_struct *tty = NULL;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&tsk->sighand->siglock, flags);
> + if (tsk->signal)
> + tty = tty_kref_get(tsk->signal->tty);
> + spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
> + return tty;
> +}
> +
> +static inline void audit_put_tty(struct tty_struct *tty)
> +{
> + tty_kref_put(tty);
> +}
I'm generally not a big fan of defining functions as inlines in header
files, with the general exception of dummy functions that will get
compiled out. Although I guess there might be some performance
argument to be made wrt audit_log_task_info().
I guess I'm fine with this, but what was the idea behind the static
inlines in audit.h? Performance, or something else?
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 195ffae..71e14d8 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1980,6 +1980,7 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
> {
> struct audit_buffer *ab;
> uid_t uid, oldloginuid, loginuid;
> + struct tty_struct *tty;
>
> if (!audit_enabled)
> return;
> @@ -1987,14 +1988,17 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
> uid = from_kuid(&init_user_ns, task_uid(current));
> oldloginuid = from_kuid(&init_user_ns, koldloginuid);
> loginuid = from_kuid(&init_user_ns, kloginuid),
> + tty = audit_get_tty(current);
>
> ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_LOGIN);
> if (!ab)
> return;
> audit_log_format(ab, "pid=%d uid=%u", task_pid_nr(current), uid);
> audit_log_task_context(ab);
> - audit_log_format(ab, " old-auid=%u auid=%u old-ses=%u ses=%u res=%d",
> - oldloginuid, loginuid, oldsessionid, sessionid, !rc);
> + audit_log_format(ab, " old-auid=%u auid=%u tty=%s old-ses=%u ses=%u res=%d",
> + oldloginuid, loginuid, tty ? tty_name(tty) : "(none)",
> + oldsessionid, sessionid, !rc);
> + audit_put_tty(tty);
> audit_log_end(ab);
> }
Because we are still using the crappy fixed string format for
kernel<->userspace communication, this patch will likely "break the
world" ... let's check with Steve but I believe the way to handle this
is to add the tty information to the end of the record.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: Python auparse bindings memory leak
From: Steve Grubb @ 2016-04-21 19:16 UTC (permalink / raw)
To: linux-audit
In-Reply-To: <CAPYiZnmdMx2GhdUcPo8x8jzUu_SrPzxgKeUFkK8Q-j_GDQ+ybw@mail.gmail.com>
Hello,
On Thursday, April 14, 2016 02:37:19 PM Santosh Ananthakrishnan wrote:
> The get_timestamp function in the auparse extension module seems to have an
> extra Py_INCREF. There's already a #FIXME at the line:
> https://fedorahosted.org/audit/browser/tags/audit-2.5.1/bindings/python/aupa
> rse_python.c#L1090 .
>
> Has this been investigated before?
I was able to confirm this is leaking memory.
> Rebuilding the package with that increment removed resolved our memory usage
> issues, so I'm hoping this doesn't silently break something.
That appears to be the fix. Thanks for reporting the issue.
-Steve
^ permalink raw reply
* [PATCH V4] audit: add tty field to LOGIN event
From: Richard Guy Briggs @ 2016-04-21 18:14 UTC (permalink / raw)
To: linux-audit, linux-kernel
Cc: Richard Guy Briggs, peter, sgrubb, pmoore, eparis
The tty field was missing from AUDIT_LOGIN events.
Refactor code to create a new function audit_get_tty(), using it to
replace the call in audit_log_task_info() and to add it to
audit_log_set_loginuid(). Lock and bump the kref to protect it, adding
audit_put_tty() alias to decrement it.
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
V4: Add missing prototype for audit_put_tty() when audit syscall is not
enabled (MIPS).
V3: Introduce audit_put_tty() alias to decrement kref.
V2: Use kref to protect tty signal struct while in use.
---
include/linux/audit.h | 24 ++++++++++++++++++++++++
kernel/audit.c | 18 +++++-------------
kernel/auditsc.c | 8 ++++++--
3 files changed, 35 insertions(+), 15 deletions(-)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index b40ed5d..32cdafb 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -26,6 +26,7 @@
#include <linux/sched.h>
#include <linux/ptrace.h>
#include <uapi/linux/audit.h>
+#include <linux/tty.h>
#define AUDIT_INO_UNSET ((unsigned long)-1)
#define AUDIT_DEV_UNSET ((dev_t)-1)
@@ -343,6 +344,23 @@ static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
return tsk->sessionid;
}
+static inline struct tty_struct *audit_get_tty(struct task_struct *tsk)
+{
+ struct tty_struct *tty = NULL;
+ unsigned long flags;
+
+ spin_lock_irqsave(&tsk->sighand->siglock, flags);
+ if (tsk->signal)
+ tty = tty_kref_get(tsk->signal->tty);
+ spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
+ return tty;
+}
+
+static inline void audit_put_tty(struct tty_struct *tty)
+{
+ tty_kref_put(tty);
+}
+
extern void __audit_ipc_obj(struct kern_ipc_perm *ipcp);
extern void __audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid, umode_t mode);
extern void __audit_bprm(struct linux_binprm *bprm);
@@ -500,6 +518,12 @@ static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
{
return -1;
}
+static inline struct tty_struct *audit_get_tty(struct task_struct *tsk)
+{
+ return NULL;
+}
+static inline void audit_put_tty(struct tty_struct *tty)
+{ }
static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
{ }
static inline void audit_ipc_set_perm(unsigned long qbytes, uid_t uid,
diff --git a/kernel/audit.c b/kernel/audit.c
index 3a3e5de..7edd776 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -64,7 +64,6 @@
#include <linux/security.h>
#endif
#include <linux/freezer.h>
-#include <linux/tty.h>
#include <linux/pid_namespace.h>
#include <net/netns/generic.h>
@@ -1873,21 +1872,14 @@ void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk)
{
const struct cred *cred;
char comm[sizeof(tsk->comm)];
- char *tty;
+ struct tty_struct *tty;
if (!ab)
return;
/* tsk == current */
cred = current_cred();
-
- spin_lock_irq(&tsk->sighand->siglock);
- if (tsk->signal && tsk->signal->tty && tsk->signal->tty->name)
- tty = tsk->signal->tty->name;
- else
- tty = "(none)";
- spin_unlock_irq(&tsk->sighand->siglock);
-
+ tty = audit_get_tty(tsk);
audit_log_format(ab,
" ppid=%d pid=%d auid=%u uid=%u gid=%u"
" euid=%u suid=%u fsuid=%u"
@@ -1903,11 +1895,11 @@ void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk)
from_kgid(&init_user_ns, cred->egid),
from_kgid(&init_user_ns, cred->sgid),
from_kgid(&init_user_ns, cred->fsgid),
- tty, audit_get_sessionid(tsk));
-
+ tty ? tty_name(tty) : "(none)",
+ audit_get_sessionid(tsk));
+ audit_put_tty(tty);
audit_log_format(ab, " comm=");
audit_log_untrustedstring(ab, get_task_comm(comm, tsk));
-
audit_log_d_path_exe(ab, tsk->mm);
audit_log_task_context(ab);
}
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 195ffae..71e14d8 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1980,6 +1980,7 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
{
struct audit_buffer *ab;
uid_t uid, oldloginuid, loginuid;
+ struct tty_struct *tty;
if (!audit_enabled)
return;
@@ -1987,14 +1988,17 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
uid = from_kuid(&init_user_ns, task_uid(current));
oldloginuid = from_kuid(&init_user_ns, koldloginuid);
loginuid = from_kuid(&init_user_ns, kloginuid),
+ tty = audit_get_tty(current);
ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_LOGIN);
if (!ab)
return;
audit_log_format(ab, "pid=%d uid=%u", task_pid_nr(current), uid);
audit_log_task_context(ab);
- audit_log_format(ab, " old-auid=%u auid=%u old-ses=%u ses=%u res=%d",
- oldloginuid, loginuid, oldsessionid, sessionid, !rc);
+ audit_log_format(ab, " old-auid=%u auid=%u tty=%s old-ses=%u ses=%u res=%d",
+ oldloginuid, loginuid, tty ? tty_name(tty) : "(none)",
+ oldsessionid, sessionid, !rc);
+ audit_put_tty(tty);
audit_log_end(ab);
}
--
1.7.1
^ permalink raw reply related
* Re: [PATCH V3] audit: add tty field to LOGIN event
From: kbuild test robot @ 2016-04-21 15:50 UTC (permalink / raw)
Cc: kbuild-all, linux-audit, linux-kernel, Richard Guy Briggs, peter,
sgrubb, pmoore, eparis
In-Reply-To: <d046ed565aa008a1d78a35409d508c187637a344.1461245943.git.rgb@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1553 bytes --]
Hi,
[auto build test ERROR on v4.6-rc4]
[also build test ERROR on next-20160421]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Richard-Guy-Briggs/audit-add-tty-field-to-LOGIN-event/20160421-233218
config: mips-allyesconfig (attached as .config)
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=mips
All errors (new ones prefixed by >>):
kernel/audit.c: In function 'audit_log_task_info':
>> kernel/audit.c:1917:2: error: implicit declaration of function 'audit_put_tty' [-Werror=implicit-function-declaration]
audit_put_tty(tty);
^
cc1: some warnings being treated as errors
vim +/audit_put_tty +1917 kernel/audit.c
1911 from_kuid(&init_user_ns, cred->fsuid),
1912 from_kgid(&init_user_ns, cred->egid),
1913 from_kgid(&init_user_ns, cred->sgid),
1914 from_kgid(&init_user_ns, cred->fsgid),
1915 tty ? tty_name(tty) : "(none)",
1916 audit_get_sessionid(tsk));
> 1917 audit_put_tty(tty);
1918 audit_log_format(ab, " comm=");
1919 audit_log_untrustedstring(ab, get_task_comm(comm, tsk));
1920 audit_log_d_path_exe(ab, tsk->mm);
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 41426 bytes --]
^ permalink raw reply
* [PATCH V3] audit: add tty field to LOGIN event
From: Richard Guy Briggs @ 2016-04-21 15:28 UTC (permalink / raw)
To: linux-audit, linux-kernel
Cc: Richard Guy Briggs, peter, sgrubb, pmoore, eparis
The tty field was missing from AUDIT_LOGIN events.
Refactor code to create a new function audit_get_tty(), using it to
replace the call in audit_log_task_info() and to add it to
audit_log_set_loginuid(). Lock and bump the kref to protect it, adding
audit_put_tty() alias to decrement it.
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
V3: Introduce audit_put_tty() alias to decrement kref.
V2: Use kref to protect tty signal struct while in use.
---
include/linux/audit.h | 22 ++++++++++++++++++++++
kernel/audit.c | 18 +++++-------------
kernel/auditsc.c | 8 ++++++--
3 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index b40ed5d..8e825b9 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -26,6 +26,7 @@
#include <linux/sched.h>
#include <linux/ptrace.h>
#include <uapi/linux/audit.h>
+#include <linux/tty.h>
#define AUDIT_INO_UNSET ((unsigned long)-1)
#define AUDIT_DEV_UNSET ((dev_t)-1)
@@ -343,6 +344,23 @@ static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
return tsk->sessionid;
}
+static inline struct tty_struct *audit_get_tty(struct task_struct *tsk)
+{
+ struct tty_struct *tty = NULL;
+ unsigned long flags;
+
+ spin_lock_irqsave(&tsk->sighand->siglock, flags);
+ if (tsk->signal)
+ tty = tty_kref_get(tsk->signal->tty);
+ spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
+ return tty;
+}
+
+static inline void audit_put_tty(struct tty_struct *tty)
+{
+ tty_kref_put(tty);
+}
+
extern void __audit_ipc_obj(struct kern_ipc_perm *ipcp);
extern void __audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid, umode_t mode);
extern void __audit_bprm(struct linux_binprm *bprm);
@@ -500,6 +518,10 @@ static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
{
return -1;
}
+static inline struct tty_struct *audit_get_tty(struct task_struct *tsk)
+{
+ return NULL;
+}
static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
{ }
static inline void audit_ipc_set_perm(unsigned long qbytes, uid_t uid,
diff --git a/kernel/audit.c b/kernel/audit.c
index 3a3e5de..7edd776 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -64,7 +64,6 @@
#include <linux/security.h>
#endif
#include <linux/freezer.h>
-#include <linux/tty.h>
#include <linux/pid_namespace.h>
#include <net/netns/generic.h>
@@ -1873,21 +1872,14 @@ void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk)
{
const struct cred *cred;
char comm[sizeof(tsk->comm)];
- char *tty;
+ struct tty_struct *tty;
if (!ab)
return;
/* tsk == current */
cred = current_cred();
-
- spin_lock_irq(&tsk->sighand->siglock);
- if (tsk->signal && tsk->signal->tty && tsk->signal->tty->name)
- tty = tsk->signal->tty->name;
- else
- tty = "(none)";
- spin_unlock_irq(&tsk->sighand->siglock);
-
+ tty = audit_get_tty(tsk);
audit_log_format(ab,
" ppid=%d pid=%d auid=%u uid=%u gid=%u"
" euid=%u suid=%u fsuid=%u"
@@ -1903,11 +1895,11 @@ void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk)
from_kgid(&init_user_ns, cred->egid),
from_kgid(&init_user_ns, cred->sgid),
from_kgid(&init_user_ns, cred->fsgid),
- tty, audit_get_sessionid(tsk));
-
+ tty ? tty_name(tty) : "(none)",
+ audit_get_sessionid(tsk));
+ audit_put_tty(tty);
audit_log_format(ab, " comm=");
audit_log_untrustedstring(ab, get_task_comm(comm, tsk));
-
audit_log_d_path_exe(ab, tsk->mm);
audit_log_task_context(ab);
}
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 195ffae..71e14d8 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1980,6 +1980,7 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
{
struct audit_buffer *ab;
uid_t uid, oldloginuid, loginuid;
+ struct tty_struct *tty;
if (!audit_enabled)
return;
@@ -1987,14 +1988,17 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
uid = from_kuid(&init_user_ns, task_uid(current));
oldloginuid = from_kuid(&init_user_ns, koldloginuid);
loginuid = from_kuid(&init_user_ns, kloginuid),
+ tty = audit_get_tty(current);
ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_LOGIN);
if (!ab)
return;
audit_log_format(ab, "pid=%d uid=%u", task_pid_nr(current), uid);
audit_log_task_context(ab);
- audit_log_format(ab, " old-auid=%u auid=%u old-ses=%u ses=%u res=%d",
- oldloginuid, loginuid, oldsessionid, sessionid, !rc);
+ audit_log_format(ab, " old-auid=%u auid=%u tty=%s old-ses=%u ses=%u res=%d",
+ oldloginuid, loginuid, tty ? tty_name(tty) : "(none)",
+ oldsessionid, sessionid, !rc);
+ audit_put_tty(tty);
audit_log_end(ab);
}
--
1.7.1
^ permalink raw reply related
* Re: New field to auditd.conf file
From: Paul Moore @ 2016-04-21 12:58 UTC (permalink / raw)
To: Deepika Sundar; +Cc: linux-audit
In-Reply-To: <CAHj_pNcuqokj9Me5hgmdp1sq9MPgomi4T5YTcvMCtU7O1s8o_A@mail.gmail.com>
As we've already mentioned several times, we can make no guarantees
regarding functionality or compatibility without seeing your code.
While it may be frustrating, this is how Open Source development
works.
If you are interested in our help you will need to describe, in
detail, what you are trying to do and ideally post your existing code
so it can be reviewed.
On Thu, Apr 21, 2016 at 1:25 AM, Deepika Sundar
<sundar.deepika18@gmail.com> wrote:
> Okay,If I update the Ausearch/aureport in order to aware of the new field in
> the audit log structure can it be feasible one?
>
> On Wed, Apr 20, 2016 at 6:00 PM, Steve Grubb <sgrubb@redhat.com> wrote:
>>
>> On Wednesday, April 20, 2016 10:05:42 AM Deepika Sundar wrote:
>> > In general way,Is there any compatibility issues if audit log structure
>> > gets modified?
>>
>> Yes, there can be problems if the log structure gets modified.
>> Ausearch/report
>> are highly optimized for an exact format.
>>
>> -Steve
>>
>>
>> > On Wed, Apr 13, 2016 at 6:01 PM, Steve Grubb <sgrubb@redhat.com> wrote:
>> > > On Wednesday, April 13, 2016 11:03:43 AM Deepika Sundar wrote:
>> > > > As per my understanding audit log structure can be extendible based
>> > > > on
>> > > > requirements and in my project I need to add the identifier field
>> > > > for
>> > > > the
>> > > > application and as of now I couldn't able to revel the What
>> > > > application
>> > > > trying to develop to update.So,Is there any possibility that without
>> > > > breaking any Compatibility issues I can do it ?
>> > >
>> > > I have no idea what you are doing so there is no guarantee that it
>> > > won't
>> > > break
>> > > something. If your project is going to be released as open source its
>> > > generally best to collaborate with people so that problems can be
>> > > pointed
>> > > out.
>> > > Otherwise you risk spending a lot of time on something only to have it
>> > > rejected.
>> > >
>> > > -Steve
>> > >
>> > > > OR If any compatibility issues please specify .
>> > > >
>> > > > On Fri, Apr 8, 2016 at 12:12 AM, Paul Moore <paul@paul-moore.com>
>> > > > wrote:
>> > > > > On Thu, Apr 7, 2016 at 12:47 AM, Deepika Sundar
>> > > > >
>> > > > > <sundar.deepika18@gmail.com> wrote:
>> > > > > > In the same way, in the kernel side
>> > > > > > Can I able to add one new field to the audit log structure
>> > > > > > without
>> > > > >
>> > > > > breaking
>> > > > >
>> > > > > > Compatibility? If so,
>> > > > > >
>> > > > > > 1.How can I add new field without breaking compatibility?
>> > > > > >
>> > > > > > or
>> > > > > >
>> > > > > > 2.Is there any reserve field in audit log structure so that I
>> > > > > > can
>> > >
>> > > make
>> > >
>> > > > > use
>> > > > >
>> > > > > > of it?
>> > > > >
>> > > > > You need to be more specific about what you are trying to do.
>> > > > > Speaking generally, unless you work to get your changed merged
>> > > > > into
>> > > > > the upstream kernel and userspace tools we cannot guarantee
>> > > > > present or
>> > > > > future compatibility.
>> > > > >
>> > > > > --
>> > > > > paul moore
>> > > > > www.paul-moore.com
>>
>
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
--
paul moore
security @ redhat
^ permalink raw reply
* Re: New field to auditd.conf file
From: Deepika Sundar @ 2016-04-21 5:25 UTC (permalink / raw)
To: Steve Grubb, linux-audit
In-Reply-To: <1515486.WT5mChES96@x2>
[-- Attachment #1.1: Type: text/plain, Size: 2515 bytes --]
Okay,If I update the Ausearch/aureport in order to aware of the new field
in the audit log structure can it be feasible one?
On Wed, Apr 20, 2016 at 6:00 PM, Steve Grubb <sgrubb@redhat.com> wrote:
> On Wednesday, April 20, 2016 10:05:42 AM Deepika Sundar wrote:
> > In general way,Is there any compatibility issues if audit log structure
> > gets modified?
>
> Yes, there can be problems if the log structure gets modified.
> Ausearch/report
> are highly optimized for an exact format.
>
> -Steve
>
>
> > On Wed, Apr 13, 2016 at 6:01 PM, Steve Grubb <sgrubb@redhat.com> wrote:
> > > On Wednesday, April 13, 2016 11:03:43 AM Deepika Sundar wrote:
> > > > As per my understanding audit log structure can be extendible based
> on
> > > > requirements and in my project I need to add the identifier field for
> > > > the
> > > > application and as of now I couldn't able to revel the What
> application
> > > > trying to develop to update.So,Is there any possibility that without
> > > > breaking any Compatibility issues I can do it ?
> > >
> > > I have no idea what you are doing so there is no guarantee that it
> won't
> > > break
> > > something. If your project is going to be released as open source its
> > > generally best to collaborate with people so that problems can be
> pointed
> > > out.
> > > Otherwise you risk spending a lot of time on something only to have it
> > > rejected.
> > >
> > > -Steve
> > >
> > > > OR If any compatibility issues please specify .
> > > >
> > > > On Fri, Apr 8, 2016 at 12:12 AM, Paul Moore <paul@paul-moore.com>
> wrote:
> > > > > On Thu, Apr 7, 2016 at 12:47 AM, Deepika Sundar
> > > > >
> > > > > <sundar.deepika18@gmail.com> wrote:
> > > > > > In the same way, in the kernel side
> > > > > > Can I able to add one new field to the audit log structure
> without
> > > > >
> > > > > breaking
> > > > >
> > > > > > Compatibility? If so,
> > > > > >
> > > > > > 1.How can I add new field without breaking compatibility?
> > > > > >
> > > > > > or
> > > > > >
> > > > > > 2.Is there any reserve field in audit log structure so that I
> can
> > >
> > > make
> > >
> > > > > use
> > > > >
> > > > > > of it?
> > > > >
> > > > > You need to be more specific about what you are trying to do.
> > > > > Speaking generally, unless you work to get your changed merged into
> > > > > the upstream kernel and userspace tools we cannot guarantee
> present or
> > > > > future compatibility.
> > > > >
> > > > > --
> > > > > paul moore
> > > > > www.paul-moore.com
>
>
[-- Attachment #1.2: Type: text/html, Size: 3879 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [PATCH V2] audit: add tty field to LOGIN event
From: Paul Moore @ 2016-04-21 1:44 UTC (permalink / raw)
To: Richard Guy Briggs; +Cc: linux-audit, linux-kernel, peter
In-Reply-To: <d9d7ad22f07986a4162a829c2add49cbe4273f34.1461089677.git.rgb@redhat.com>
On Wed, Apr 20, 2016 at 7:31 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index b40ed5d..b00beef 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -26,6 +26,7 @@
> #include <linux/sched.h>
> #include <linux/ptrace.h>
> #include <uapi/linux/audit.h>
> +#include <linux/tty.h>
>
> #define AUDIT_INO_UNSET ((unsigned long)-1)
> #define AUDIT_DEV_UNSET ((dev_t)-1)
> @@ -343,6 +344,18 @@ static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
> return tsk->sessionid;
> }
>
> +static inline struct tty_struct *audit_get_tty(struct task_struct *tsk)
> +{
> + struct tty_struct *tty = NULL;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&tsk->sighand->siglock, flags);
> + if (tsk->signal)
> + tty = tty_kref_get(tsk->signal->tty);
> + spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
> + return tty;
> +}
I'll look at this patch closer tomorrow, but one thing jumped out at
me just now: we should probably have a audit_put_tty(...) to match the
audit_get_tty(...). It seems more obvious than trying to match
audit_get_tty() and tty_kref_put() in a function.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* [PATCH V2] audit: add tty field to LOGIN event
From: Richard Guy Briggs @ 2016-04-20 23:31 UTC (permalink / raw)
To: linux-audit, linux-kernel
Cc: Richard Guy Briggs, peter, sgrubb, pmoore, eparis
The tty field was missing from AUDIT_LOGIN events.
Refactor code to create a new function audit_get_tty(), using it to
replace the call in audit_log_task_info() and to add it to
audit_log_set_loginuid(). Lock and bump the kref to protect it.
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
V2: Use kref to protect tty signal struct while in use.
---
include/linux/audit.h | 17 +++++++++++++++++
kernel/audit.c | 18 +++++-------------
kernel/auditsc.c | 8 ++++++--
3 files changed, 28 insertions(+), 15 deletions(-)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index b40ed5d..b00beef 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -26,6 +26,7 @@
#include <linux/sched.h>
#include <linux/ptrace.h>
#include <uapi/linux/audit.h>
+#include <linux/tty.h>
#define AUDIT_INO_UNSET ((unsigned long)-1)
#define AUDIT_DEV_UNSET ((dev_t)-1)
@@ -343,6 +344,18 @@ static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
return tsk->sessionid;
}
+static inline struct tty_struct *audit_get_tty(struct task_struct *tsk)
+{
+ struct tty_struct *tty = NULL;
+ unsigned long flags;
+
+ spin_lock_irqsave(&tsk->sighand->siglock, flags);
+ if (tsk->signal)
+ tty = tty_kref_get(tsk->signal->tty);
+ spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
+ return tty;
+}
+
extern void __audit_ipc_obj(struct kern_ipc_perm *ipcp);
extern void __audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid, umode_t mode);
extern void __audit_bprm(struct linux_binprm *bprm);
@@ -500,6 +513,10 @@ static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
{
return -1;
}
+static inline struct tty_struct *audit_get_tty(struct task_struct *tsk)
+{
+ return NULL;
+}
static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
{ }
static inline void audit_ipc_set_perm(unsigned long qbytes, uid_t uid,
diff --git a/kernel/audit.c b/kernel/audit.c
index 3a3e5de..4368ed1 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -64,7 +64,6 @@
#include <linux/security.h>
#endif
#include <linux/freezer.h>
-#include <linux/tty.h>
#include <linux/pid_namespace.h>
#include <net/netns/generic.h>
@@ -1873,21 +1872,14 @@ void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk)
{
const struct cred *cred;
char comm[sizeof(tsk->comm)];
- char *tty;
+ struct tty_struct *tty;
if (!ab)
return;
/* tsk == current */
cred = current_cred();
-
- spin_lock_irq(&tsk->sighand->siglock);
- if (tsk->signal && tsk->signal->tty && tsk->signal->tty->name)
- tty = tsk->signal->tty->name;
- else
- tty = "(none)";
- spin_unlock_irq(&tsk->sighand->siglock);
-
+ tty = audit_get_tty(tsk);
audit_log_format(ab,
" ppid=%d pid=%d auid=%u uid=%u gid=%u"
" euid=%u suid=%u fsuid=%u"
@@ -1903,11 +1895,11 @@ void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk)
from_kgid(&init_user_ns, cred->egid),
from_kgid(&init_user_ns, cred->sgid),
from_kgid(&init_user_ns, cred->fsgid),
- tty, audit_get_sessionid(tsk));
-
+ tty ? tty_name(tty) : "(none)",
+ audit_get_sessionid(tsk));
+ tty_kref_put(tty);
audit_log_format(ab, " comm=");
audit_log_untrustedstring(ab, get_task_comm(comm, tsk));
-
audit_log_d_path_exe(ab, tsk->mm);
audit_log_task_context(ab);
}
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 195ffae..9d7c4cd 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1980,6 +1980,7 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
{
struct audit_buffer *ab;
uid_t uid, oldloginuid, loginuid;
+ struct tty_struct *tty;
if (!audit_enabled)
return;
@@ -1987,14 +1988,17 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
uid = from_kuid(&init_user_ns, task_uid(current));
oldloginuid = from_kuid(&init_user_ns, koldloginuid);
loginuid = from_kuid(&init_user_ns, kloginuid),
+ tty = audit_get_tty(current);
ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_LOGIN);
if (!ab)
return;
audit_log_format(ab, "pid=%d uid=%u", task_pid_nr(current), uid);
audit_log_task_context(ab);
- audit_log_format(ab, " old-auid=%u auid=%u old-ses=%u ses=%u res=%d",
- oldloginuid, loginuid, oldsessionid, sessionid, !rc);
+ audit_log_format(ab, " old-auid=%u auid=%u tty=%s old-ses=%u ses=%u res=%d",
+ oldloginuid, loginuid, tty ? tty_name(tty) : "(none)",
+ oldsessionid, sessionid, !rc);
+ tty_kref_put(tty);
audit_log_end(ab);
}
--
1.7.1
^ permalink raw reply related
* Re: PID's Mapping
From: Steve Grubb @ 2016-04-20 12:33 UTC (permalink / raw)
To: Deepika Sundar, linux-audit
In-Reply-To: <CAHj_pNfPdsq32TNC-jET57sTJ03WpB1bnNgVrX_fYTwJTpgjQQ@mail.gmail.com>
On Wednesday, April 20, 2016 10:06:38 AM Deepika Sundar wrote:
> Is there any way that can be suggested as to map PID's of namespace in
> global?
This is on the TODO list. We have been kicking around several ideas but have
not come to a conclusion about what exactly needs to be done. The upshot of
this is that basically containers have no support.
-Steve
> On Mon, Apr 18, 2016 at 8:47 PM, Paul Moore <paul@paul-moore.com> wrote:
> > Please ask your question on the mailing list so that everyone can benefit.
> >
> > On Mon, Apr 18, 2016 at 1:34 AM, Deepika Sundar
> >
> > <sundar.deepika18@gmail.com> wrote:
> > > How it can be achieved ,Can I get any idea on this?
> > >
> > > On Fri, Apr 15, 2016 at 4:12 AM, Paul Moore <paul@paul-moore.com> wrote:
> > >> On Wed, Apr 13, 2016 at 1:43 AM, sowndarya kumar
> > >>
> > >> <sowndarya.nadar@gmail.com> wrote:
> > >> > Hi
> > >> >
> > >> > Is there any way to map the PID's seen in the namespace application
> >
> > with
> >
> > >> > the
> > >> > PID's seen in global?
> > >> > If it can be done please provide the documentation or idea on how it
> >
> > can
> >
> > >> > be
> > >> > done.
> > >>
> > >> In general the audit subsystem doesn't pay attention to namespaces,
> > >> all PIDs reported to userspace are reported with respect to the init
> > >> namespace.
> > >>
> > >> --
> > >> paul moore
> > >> www.paul-moore.com
> > >>
> > >> --
> > >> Linux-audit mailing list
> > >> Linux-audit@redhat.com
> > >> https://www.redhat.com/mailman/listinfo/linux-audit
> >
> > --
> > paul moore
> > www.paul-moore.com
^ permalink raw reply
* Re: New field to auditd.conf file
From: Steve Grubb @ 2016-04-20 12:30 UTC (permalink / raw)
To: Deepika Sundar, linux-audit
In-Reply-To: <CAHj_pNf9BZxyAzgpBLjtCVpKHSQqHxHV44iy+21JEc93jW6cRQ@mail.gmail.com>
On Wednesday, April 20, 2016 10:05:42 AM Deepika Sundar wrote:
> In general way,Is there any compatibility issues if audit log structure
> gets modified?
Yes, there can be problems if the log structure gets modified. Ausearch/report
are highly optimized for an exact format.
-Steve
> On Wed, Apr 13, 2016 at 6:01 PM, Steve Grubb <sgrubb@redhat.com> wrote:
> > On Wednesday, April 13, 2016 11:03:43 AM Deepika Sundar wrote:
> > > As per my understanding audit log structure can be extendible based on
> > > requirements and in my project I need to add the identifier field for
> > > the
> > > application and as of now I couldn't able to revel the What application
> > > trying to develop to update.So,Is there any possibility that without
> > > breaking any Compatibility issues I can do it ?
> >
> > I have no idea what you are doing so there is no guarantee that it won't
> > break
> > something. If your project is going to be released as open source its
> > generally best to collaborate with people so that problems can be pointed
> > out.
> > Otherwise you risk spending a lot of time on something only to have it
> > rejected.
> >
> > -Steve
> >
> > > OR If any compatibility issues please specify .
> > >
> > > On Fri, Apr 8, 2016 at 12:12 AM, Paul Moore <paul@paul-moore.com> wrote:
> > > > On Thu, Apr 7, 2016 at 12:47 AM, Deepika Sundar
> > > >
> > > > <sundar.deepika18@gmail.com> wrote:
> > > > > In the same way, in the kernel side
> > > > > Can I able to add one new field to the audit log structure without
> > > >
> > > > breaking
> > > >
> > > > > Compatibility? If so,
> > > > >
> > > > > 1.How can I add new field without breaking compatibility?
> > > > >
> > > > > or
> > > > >
> > > > > 2.Is there any reserve field in audit log structure so that I can
> >
> > make
> >
> > > > use
> > > >
> > > > > of it?
> > > >
> > > > You need to be more specific about what you are trying to do.
> > > > Speaking generally, unless you work to get your changed merged into
> > > > the upstream kernel and userspace tools we cannot guarantee present or
> > > > future compatibility.
> > > >
> > > > --
> > > > paul moore
> > > > www.paul-moore.com
^ permalink raw reply
* Re: PID's Mapping
From: Deepika Sundar @ 2016-04-20 4:36 UTC (permalink / raw)
To: Paul Moore, linux-audit, Steve Grubb
In-Reply-To: <CAHC9VhR=S5DP-DUMxizNLr4RwP8XD6-EPStCQU5sQbQVXg_Qjw@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 1206 bytes --]
Is there any way that can be suggested as to map PID's of namespace in
global?
On Mon, Apr 18, 2016 at 8:47 PM, Paul Moore <paul@paul-moore.com> wrote:
> Please ask your question on the mailing list so that everyone can benefit.
>
> On Mon, Apr 18, 2016 at 1:34 AM, Deepika Sundar
> <sundar.deepika18@gmail.com> wrote:
> > How it can be achieved ,Can I get any idea on this?
> >
> > On Fri, Apr 15, 2016 at 4:12 AM, Paul Moore <paul@paul-moore.com> wrote:
> >>
> >> On Wed, Apr 13, 2016 at 1:43 AM, sowndarya kumar
> >> <sowndarya.nadar@gmail.com> wrote:
> >> > Hi
> >> >
> >> > Is there any way to map the PID's seen in the namespace application
> with
> >> > the
> >> > PID's seen in global?
> >> > If it can be done please provide the documentation or idea on how it
> can
> >> > be
> >> > done.
> >>
> >> In general the audit subsystem doesn't pay attention to namespaces,
> >> all PIDs reported to userspace are reported with respect to the init
> >> namespace.
> >>
> >> --
> >> paul moore
> >> www.paul-moore.com
> >>
> >> --
> >> Linux-audit mailing list
> >> Linux-audit@redhat.com
> >> https://www.redhat.com/mailman/listinfo/linux-audit
> >
> >
>
>
>
> --
> paul moore
> www.paul-moore.com
>
[-- Attachment #1.2: Type: text/html, Size: 2422 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: New field to auditd.conf file
From: Deepika Sundar @ 2016-04-20 4:35 UTC (permalink / raw)
To: Steve Grubb, Paul Moore, linux-audit
In-Reply-To: <9022906.fX42xjMDC3@x2>
[-- Attachment #1.1: Type: text/plain, Size: 1877 bytes --]
In general way,Is there any compatibility issues if audit log structure
gets modified?
On Wed, Apr 13, 2016 at 6:01 PM, Steve Grubb <sgrubb@redhat.com> wrote:
> On Wednesday, April 13, 2016 11:03:43 AM Deepika Sundar wrote:
> > As per my understanding audit log structure can be extendible based on
> > requirements and in my project I need to add the identifier field for the
> > application and as of now I couldn't able to revel the What application
> > trying to develop to update.So,Is there any possibility that without
> > breaking any Compatibility issues I can do it ?
>
> I have no idea what you are doing so there is no guarantee that it won't
> break
> something. If your project is going to be released as open source its
> generally best to collaborate with people so that problems can be pointed
> out.
> Otherwise you risk spending a lot of time on something only to have it
> rejected.
>
> -Steve
>
>
> > OR If any compatibility issues please specify .
> >
> > On Fri, Apr 8, 2016 at 12:12 AM, Paul Moore <paul@paul-moore.com> wrote:
> > > On Thu, Apr 7, 2016 at 12:47 AM, Deepika Sundar
> > >
> > > <sundar.deepika18@gmail.com> wrote:
> > > > In the same way, in the kernel side
> > > > Can I able to add one new field to the audit log structure without
> > >
> > > breaking
> > >
> > > > Compatibility? If so,
> > > >
> > > > 1.How can I add new field without breaking compatibility?
> > > >
> > > > or
> > > >
> > > > 2.Is there any reserve field in audit log structure so that I can
> make
> > >
> > > use
> > >
> > > > of it?
> > >
> > > You need to be more specific about what you are trying to do.
> > > Speaking generally, unless you work to get your changed merged into
> > > the upstream kernel and userspace tools we cannot guarantee present or
> > > future compatibility.
> > >
> > > --
> > > paul moore
> > > www.paul-moore.com
>
>
[-- Attachment #1.2: Type: text/html, Size: 2851 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [PATCH] audit: add tty field to LOGIN event
From: Richard Guy Briggs @ 2016-04-18 18:27 UTC (permalink / raw)
To: Peter Hurley; +Cc: linux-audit, linux-kernel, sgrubb, pmoore, eparis
In-Reply-To: <570EE4D4.4080903@hurleysoftware.com>
On 16/04/13, Peter Hurley wrote:
> Hi Richard,
Hi Peter,
> On 04/13/2016 04:25 PM, Richard Guy Briggs wrote:
> > The tty field was missing from AUDIT_LOGIN events.
> >
> > Refactor code to create a new function audit_get_tty(), using it to
> > replace the call in audit_log_task_info() and to add it to
> > audit_log_set_loginuid().
> >
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> > include/linux/audit.h | 18 ++++++++++++++++++
> > kernel/audit.c | 11 +----------
> > kernel/auditsc.c | 5 +++--
> > 3 files changed, 22 insertions(+), 12 deletions(-)
> >
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index b40ed5d..20c6649 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -26,6 +26,7 @@
> > #include <linux/sched.h>
> > #include <linux/ptrace.h>
> > #include <uapi/linux/audit.h>
> > +#include <linux/tty.h>
> >
> > #define AUDIT_INO_UNSET ((unsigned long)-1)
> > #define AUDIT_DEV_UNSET ((dev_t)-1)
> > @@ -343,6 +344,19 @@ static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
> > return tsk->sessionid;
> > }
> >
> > +static inline char *audit_get_tty(struct task_struct *tsk)
> > +{
> > + char *tty;
> > +
> > + spin_lock_irq(&tsk->sighand->siglock);
> > + if (tsk->signal && tsk->signal->tty && tsk->signal->tty->name)
> > + tty = tsk->signal->tty->name;
> > + else
> > + tty = "(none)";
> > + spin_unlock_irq(&tsk->sighand->siglock);
>
> This is unsafe because the tty could be immediately torn down after the
> siglock is dropped, and return a dangling ptr.
Understood. The other option is to copy the value out...
Thanks for the helpful review. Rev 2 coming...
> > + return tty;
> > +}
> > +
> > extern void __audit_ipc_obj(struct kern_ipc_perm *ipcp);
> > extern void __audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid, umode_t mode);
> > extern void __audit_bprm(struct linux_binprm *bprm);
> > @@ -500,6 +514,10 @@ static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
> > {
> > return -1;
> > }
> > +static inline char *audit_get_tty(struct task_struct *tsk)
> > +{
> > + return "(invalid)";
> > +}
> > static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
> > { }
> > static inline void audit_ipc_set_perm(unsigned long qbytes, uid_t uid,
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > index 3a3e5de..fae11df 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -64,7 +64,6 @@
> > #include <linux/security.h>
> > #endif
> > #include <linux/freezer.h>
> > -#include <linux/tty.h>
> > #include <linux/pid_namespace.h>
> > #include <net/netns/generic.h>
> >
> > @@ -1873,7 +1872,6 @@ void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk)
> > {
> > const struct cred *cred;
> > char comm[sizeof(tsk->comm)];
> > - char *tty;
>
> struct tty_struct *tty;
>
> >
> > if (!ab)
> > return;
> > @@ -1881,13 +1879,6 @@ void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk)
> > /* tsk == current */
> > cred = current_cred();
> >
> > - spin_lock_irq(&tsk->sighand->siglock);
> > - if (tsk->signal && tsk->signal->tty && tsk->signal->tty->name)
> > - tty = tsk->signal->tty->name;
> > - else
> > - tty = "(none)";
> > - spin_unlock_irq(&tsk->sighand->siglock);
>
> tty = get_current_tty();
>
> > -
> > audit_log_format(ab,
> > " ppid=%d pid=%d auid=%u uid=%u gid=%u"
> > " euid=%u suid=%u fsuid=%u"
> > @@ -1903,7 +1894,7 @@ void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk)
> > from_kgid(&init_user_ns, cred->egid),
> > from_kgid(&init_user_ns, cred->sgid),
> > from_kgid(&init_user_ns, cred->fsgid),
> > - tty, audit_get_sessionid(tsk));
> > + audit_get_tty(tsk), audit_get_sessionid(tsk));
>
> tty_name(tty), ....);
> ^^^^^^^^^^
> returns "NULL tty" if tty == NULL
>
> tty_kref_put(tty);
>
>
> >
> > audit_log_format(ab, " comm=");
> > audit_log_untrustedstring(ab, get_task_comm(comm, tsk));
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index 195ffae..a0467fb 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -1993,8 +1993,9 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
> > return;
> > audit_log_format(ab, "pid=%d uid=%u", task_pid_nr(current), uid);
> > audit_log_task_context(ab);
> > - audit_log_format(ab, " old-auid=%u auid=%u old-ses=%u ses=%u res=%d",
> > - oldloginuid, loginuid, oldsessionid, sessionid, !rc);
>
> tty = get_current_tty();
>
> > + audit_log_format(ab, " old-auid=%u auid=%u tty=%s old-ses=%u ses=%u res=%d",
> > + oldloginuid, loginuid, audit_get_tty(current),
>
> ......., tty_name(tty),
>
> > + oldsessionid, sessionid, !rc);
>
> tty_kref_put(tty);
>
> Regards,
> Peter Hurley
>
>
> > audit_log_end(ab);
> > }
> >
> >
>
- RGB
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox