Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

2017-11-20 Thread Petr Mladek
On Mon 2017-11-13 11:16:28, kaiwan.billimo...@gmail.com wrote:
> On Mon, 2017-11-13 at 09:21 +1100, Tobin C. Harding wrote:
> > On Fri, Nov 10, 2017 at 07:26:34PM +0530, kaiwan.billimo...@gmail.com
> > >  - it currently hard-codes a global 'PAGE_OFFSET_32BIT=0xc000'
> > > , just
> > >  so I can test quickly; must figure whether to query it or pass it;
> > >  Suggestions?
> > 
> > Perhaps we should have a command line option for this.
> > 
> > --kernel-base-address
> 
> Why not just detect it programatically? We could devise a series of
> fallbacks; something like:
> - if .config exists in the kernel source tree root, grep it for
> PAGE_OFFSET
> - if not, grep the arch-specific (arch//configs/)
> for the same
> - if for some reason we don't have enough info regarding specific
> platform and thus the defconfig filename (could happen for ARM, PPC?),
> we then fail and request the user to pass it as a parameter.

You might also check /proc/config.gz.

Best Regards,
Petr


Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

2017-11-19 Thread Tobin C. Harding
On Fri, Nov 10, 2017 at 07:26:34PM +0530, kaiwan.billimo...@gmail.com wrote:
> On Tue, 2017-11-07 at 21:32 +1100, Tobin C. Harding wrote:
[snip]

> Finally, unsure if am working against the latest ver of your script Tobin, 
> apologies if not.

The latest version of leaking_addresses.pl is now in Linus' tree.

thanks,
Tobin.


Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

2017-11-12 Thread Kaiwan N Billimoria
On Mon, Nov 13, 2017 at 11:38 AM, Tobin C. Harding  wrote:
> On Mon, Nov 13, 2017 at 11:16:28AM +0530, kaiwan.billimo...@gmail.com wrote:
>> On Mon, 2017-11-13 at 09:21 +1100, Tobin C. Harding wrote:
>> > On Fri, Nov 10, 2017 at 07:26:34PM +0530, kaiwan.billimo...@gmail.com
>> >  wrote:
>> > > On Tue, 2017-11-07 at 21:32 +1100, Tobin C. Harding wrote:
>> > > > Currently we are leaking addresses from the kernel to user space.
>> > > > This
...
>
> So, Linus has requested that I set up a tree for the development of
> this. I have to work out the details of how to do that and then I'll
> email you so you can get the pull the current version. I can then take
> your patch via LKML as per usual.
>
Super. Thanks.


Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

2017-11-12 Thread Tobin C. Harding
On Mon, Nov 13, 2017 at 11:16:28AM +0530, kaiwan.billimo...@gmail.com wrote:
> On Mon, 2017-11-13 at 09:21 +1100, Tobin C. Harding wrote:
> > On Fri, Nov 10, 2017 at 07:26:34PM +0530, kaiwan.billimo...@gmail.com
> >  wrote:
> > > On Tue, 2017-11-07 at 21:32 +1100, Tobin C. Harding wrote:
> > > > Currently we are leaking addresses from the kernel to user space.
> > > > This
> > > > script is an attempt to find some of those leakages. Script
> > > > parses
> > > > `dmesg` output and /proc and /sys files for hex strings that look
> > > > like
> > > > kernel addresses.
> > > > 
> > > > Only works for 64 bit kernels, the reason being that kernel
> > > > addresses
> > > > on 64 bit kernels have '' as the leading bit pattern making
> > > > greping
> > > > possible. On 32 kernels we don't have this luxury.
> > > 
> > > Tobin C. Harding  wrote:
> > > > Only works for 64 bit kernels, the reason being that kernel
> > > > addresses
> > > > on 64 bit kernels have '' as the leading bit pattern making
> > > > greping
> > > > possible. On 32 kernels we don't have this luxury.
> > > 
> > > [RFC] leaking_addresses.pl - enhance it to work for 32-bit kernels
> > > as well
> > > 
> > > (Firstly, apologies if I've got the protocol horribly wrong- should
> > > this
> > > be a new thread altogether?)
> > 
> > I think this patch will need to wait until the patch set that is
> > currently in flight is either merged or dropped.
> > 
> Thanks for looking at it!
> Okay; blocking on merge || drop...  :-)

So, Linus has requested that I set up a tree for the development of
this. I have to work out the details of how to do that and then I'll
email you so you can get the pull the current version. I can then take
your patch via LKML as per usual.

> > We can work this out pragmatically, Perl can give us an architecture
> > string then a few regexs can ascertain which architecture we are
> > running
> > on. This is in the inflight patch set. 
> > 
> > > The patch below does Not take into account (yet) stuff like:
> > >  - exactly which files & dirs should be skipped on 32-bit (will it
> > > be
> > > identical to 64-bit?; unsure..)
> > 
> > As per discussion later in this thread we may need to consider
> > architecture specific lists for files/directories to skip. 
> Right
> > 
> > >  - it currently hard-codes a global 'PAGE_OFFSET_32BIT=0xc000'
> > > , just
> > >  so I can test quickly; must figure whether to query it or pass it;
> > >  Suggestions?
> > 
> > Perhaps we should have a command line option for this.
> > 
> > --kernel-base-address
> 
> Why not just detect it programatically? We could devise a series of
> fallbacks; something like:
> - if .config exists in the kernel source tree root, grep it for
> PAGE_OFFSET
> - if not, grep the arch-specific (arch//configs/)
> for the same
> - if for some reason we don't have enough info regarding specific
> platform and thus the defconfig filename (could happen for ARM, PPC?),
> we then fail and request the user to pass it as a parameter.
> 
> > >  - the 'false positives'; again, what differs for 32-bit?

Sounds good to me.

thanks,
Tobin.


Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

2017-11-12 Thread kaiwan . billimoria
On Mon, 2017-11-13 at 09:21 +1100, Tobin C. Harding wrote:
> On Fri, Nov 10, 2017 at 07:26:34PM +0530, kaiwan.billimo...@gmail.com
>  wrote:
> > On Tue, 2017-11-07 at 21:32 +1100, Tobin C. Harding wrote:
> > > Currently we are leaking addresses from the kernel to user space.
> > > This
> > > script is an attempt to find some of those leakages. Script
> > > parses
> > > `dmesg` output and /proc and /sys files for hex strings that look
> > > like
> > > kernel addresses.
> > > 
> > > Only works for 64 bit kernels, the reason being that kernel
> > > addresses
> > > on 64 bit kernels have '' as the leading bit pattern making
> > > greping
> > > possible. On 32 kernels we don't have this luxury.
> > 
> > Tobin C. Harding  wrote:
> > > Only works for 64 bit kernels, the reason being that kernel
> > > addresses
> > > on 64 bit kernels have '' as the leading bit pattern making
> > > greping
> > > possible. On 32 kernels we don't have this luxury.
> > 
> > [RFC] leaking_addresses.pl - enhance it to work for 32-bit kernels
> > as well
> > 
> > (Firstly, apologies if I've got the protocol horribly wrong- should
> > this
> > be a new thread altogether?)
> 
> I think this patch will need to wait until the patch set that is
> currently in flight is either merged or dropped.
> 
Thanks for looking at it!
Okay; blocking on merge || drop...  :-)

> > 
> We can work this out pragmatically, Perl can give us an architecture
> string then a few regexs can ascertain which architecture we are
> running
> on. This is in the inflight patch set. 
> 
> > The patch below does Not take into account (yet) stuff like:
> >  - exactly which files & dirs should be skipped on 32-bit (will it
> > be
> > identical to 64-bit?; unsure..)
> 
> As per discussion later in this thread we may need to consider
> architecture specific lists for files/directories to skip. 
Right
> 
> >  - it currently hard-codes a global 'PAGE_OFFSET_32BIT=0xc000'
> > , just
> >  so I can test quickly; must figure whether to query it or pass it;
> >  Suggestions?
> 
> Perhaps we should have a command line option for this.
> 
>   --kernel-base-address

Why not just detect it programatically? We could devise a series of
fallbacks; something like:
- if .config exists in the kernel source tree root, grep it for
PAGE_OFFSET
- if not, grep the arch-specific (arch//configs/)
for the same
- if for some reason we don't have enough info regarding specific
platform and thus the defconfig filename (could happen for ARM, PPC?),
we then fail and request the user to pass it as a parameter.

> >  - the 'false positives'; again, what differs for 32-bit?
> >(BTW, shouldn't the dmesg 'root=UUID=<...>' line be a false
> > positive
> > & skipped?).
> 
> We could probably do with architecture specific false
> positives. Inflight patch set refactors false_positive() so adding to
> this should be easy.
Sure.
> 
> > Also, I must point out that I'm a complete newbie to Perl :-) so,
> > pl excuse
> > my highly inadequate perl-foo; I rely on you perl gurus out there
> > to fix
> > and optimize :)
> 
> I'm no Perl guru but following are a few tips I have picked up over
> the
> last month.
Thanks, will fix the issues you point out..
> 
> > 
> Conceptually your ideas look good to me. If there is some reason this
> approach won't work hopefully someone else will jump in and say so.
> 
> Nice work, thanks for putting in effort to get 32 bit machines
> supported. Let's see what happens with the inflight patch set then
> work
> on getting these ideas in.
> 
Thanks! yes..
> thanks,
> Tobin.


Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

2017-11-12 Thread Michael Ellerman
Frank Rowand  writes:
> Hi Michael,
>
> On 11/12/17 03:49, Michael Ellerman wrote:
...
>> 
>> On our bare metal machines the device tree comes from skiboot
>> (firmware), with some of the content provided by hostboot (other
>> firmware), both of which are open source, so in theory most of the
>> information is available in *some* source tree. But there's still
>> information about runtime allocations etc. that is not available in the
>> source anywhere.
>
> Thanks for the additional information. 
>
> Can you explain a little bit what "runtime allocations" are?  Are you
> referring to the memory reservation block, the memory node(s) and the
> chosen node?  Or other information?

Yeah I was thinking of memory reservations. They're under the
reserved-memory node as well as the reservation block, eg:

$ ls -1 /proc/device-tree/reserved-memory/
ibm,firmware-allocs-memory@10
ibm,firmware-allocs-memory@18
ibm,firmware-allocs-memory@39c0
ibm,firmware-allocs-memory@8
ibm,firmware-code@3000
ibm,firmware-data@3100
ibm,firmware-heap@3030
ibm,firmware-stacks@31c0
ibm,hbrt-code-image@1ffd51
ibm,hbrt-target-image@1ffd6a
ibm,hbrt-vpd-image@1ffd70
ibm,slw-image@1ffda0
ibm,slw-image@1ffde0
ibm,slw-image@1ffe20
ibm,slw-image@1ffe60


There's also some new systems where a catalog of PMU events is stored in
flash as a DTB and then stitched into the device tree by skiboot before
booting Linux.

Anyway my point was mainly just that the device tree is not simply a
copy of something in the kernel source.

cheers


Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

2017-11-12 Thread Tobin C. Harding
On Fri, Nov 10, 2017 at 07:26:34PM +0530, kaiwan.billimo...@gmail.com wrote:
> On Tue, 2017-11-07 at 21:32 +1100, Tobin C. Harding wrote:
> > Currently we are leaking addresses from the kernel to user space.
> > This
> > script is an attempt to find some of those leakages. Script parses
> > `dmesg` output and /proc and /sys files for hex strings that look
> > like
> > kernel addresses.
> > 
> > Only works for 64 bit kernels, the reason being that kernel addresses
> > on 64 bit kernels have '' as the leading bit pattern making
> > greping
> > possible. On 32 kernels we don't have this luxury.
> 
> Tobin C. Harding  wrote:
> >Only works for 64 bit kernels, the reason being that kernel addresses
> >on 64 bit kernels have '' as the leading bit pattern making greping
> >possible. On 32 kernels we don't have this luxury.
> 
> [RFC] leaking_addresses.pl - enhance it to work for 32-bit kernels as well
> 
> (Firstly, apologies if I've got the protocol horribly wrong- should this
> be a new thread altogether?)

I think this patch will need to wait until the patch set that is
currently in flight is either merged or dropped.

> Ok so, I was interested in figuring - why not have this useful script work
> for 32-bit kernel virtual addresses as well (and those systems by
> extension).

Awesome.

> The approach am considering, pl correct me if I'm way off:
> on 32-bit, the kernel macro PAGE_OFFSET will give us the user-kernel split;
> (alternatively, could also script up CONFIG_VMSPLIT_[n]G and figure the
> split from there.)
> 
> For the time being, lets say we go with the "use PAGE_OFFSET" approach and
> PAGE_OFFSET = 0xc000 , whch implies we have a 3:1 GB user:kernel split.
> So any virtual addresses >= PAGE_OFFSET are kernel virtual addresses (i
> know, untrue on some ARM-32 systems!).
> 
> As a very early and *far-from-perfect* start, I've enhanced Tobin's Perl
> script to take into account 32-bit address space by passing the
> parameter '--bit-size='.

We can work this out pragmatically, Perl can give us an architecture
string then a few regexs can ascertain which architecture we are running
on. This is in the inflight patch set. 

> The patch below does Not take into account (yet) stuff like:
>  - exactly which files & dirs should be skipped on 32-bit (will it be
> identical to 64-bit?; unsure..)

As per discussion later in this thread we may need to consider
architecture specific lists for files/directories to skip. 

>  - it currently hard-codes a global 'PAGE_OFFSET_32BIT=0xc000' , just
>  so I can test quickly; must figure whether to query it or pass it;
>  Suggestions?

Perhaps we should have a command line option for this.

--kernel-base-address

>  - the 'false positives'; again, what differs for 32-bit?
>(BTW, shouldn't the dmesg 'root=UUID=<...>' line be a false positive
> & skipped?).

We could probably do with architecture specific false
positives. Inflight patch set refactors false_positive() so adding to
this should be easy.

> Also, I must point out that I'm a complete newbie to Perl :-) so, pl excuse
> my highly inadequate perl-foo; I rely on you perl gurus out there to fix
> and optimize :)

I'm no Perl guru but following are a few tips I have picked up over the
last month.

> Yes, I've **Very Minimally** tested the patch in it's current form on:
> a) a regular (Fedora 26) x86_64 desktop,
> b) a (Debian 7) 32-bit kernel (VM) with PAGE_OFFSET=3 Gb
> and it seems all right, considering...
> 
> Some sample output from test (b), if interested:
> =
> dmesg: [0.00] found SMP MP-table at [c00f1280] f1280
> dmesg: [0.00] Base memory trampoline at [c009b000] 9b000 size 16384
> dmesg: [0.00] ACPI: Local APIC address 0xfee0
> dmesg: [0.00] free_area_init_node: node 0, pgdat c1418bc0, 
> node_mem_map dfbfa200
> dmesg: [0.00] ACPI: Local APIC address 0xfee0
> dmesg: [0.00] ACPI: IOAPIC (id[0x00] address[0xfec0] gsi_base[0])
> dmesg: [0.00] IOAPIC[0]: apic_id 0, version 17, address 0xfec0, 
> GSI 0-23
> dmesg: [0.00] PERCPU: Embedded 14 pages/cpu @dfbe8000 s33344 r0 
> d24000 u57344
> dmesg: [0.00] fixmap  : 0xffd36000 - 0xf000   (2852 kB)
> dmesg: [0.00] pkmap   : 0xffa0 - 0xffc0   (2048 kB)
> dmesg: [0.00] vmalloc : 0xe07fb000 - 0xff9fe000   ( 498 MB)
> dmesg: [0.00] lowmem  : 0xc000 - 0xdfffb000   ( 511 MB)
> dmesg: [0.00]   .init : 0xc1421000 - 0xc148c000   ( 428 kB)
> 
> [...]
> 
> /proc/kallsyms: c10010e8 T _stext
> /proc/kallsyms: c1002000 T hypercall_page
> /proc/kallsyms: c1003000 t arch_local_save_flags
> /proc/kallsyms: c1003007 t arch_local_irq_enable
> /proc/kallsyms: c100300e T do_one_initcall
> 
> << ... plenty more kallsyms of course (92.5% of the output to be precise!) 
> ... >>
> 
> /proc/modules: loop 17803 0 - Live 0xe097c000
> /proc/modules: crc32c_intel 12659 0 - Live 0xe096e000
> /proc/modules: snd_pcm

Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

2017-11-12 Thread Tobin C. Harding
On Sun, Nov 12, 2017 at 10:02:55AM -0800, Frank Rowand wrote:
> Hi Michael,
> 
> On 11/12/17 03:49, Michael Ellerman wrote:
> > Hi Frank,
> > 
> > Frank Rowand  writes:
> >> Hi Michael, Tobin,
> >>
> >> On 11/08/17 04:10, Michael Ellerman wrote:
> >>> "Tobin C. Harding"  writes:
>  Currently we are leaking addresses from the kernel to user space. This
>  script is an attempt to find some of those leakages. Script parses
>  `dmesg` output and /proc and /sys files for hex strings that look like
>  kernel addresses.
> 
>  Only works for 64 bit kernels, the reason being that kernel addresses
>  on 64 bit kernels have '' as the leading bit pattern making greping
>  possible.
> >>>
> >>> That doesn't work super well on other architectures :D
> >>>
> >>> I don't speak perl but presumably you can check the arch somehow and
> >>> customise the regex?
> >>>
> >>> ...
>  +# Return _all_ non false positive addresses from $line.
>  +sub extract_addresses
>  +{
>  +my ($line) = @_;
>  +my $address = '\b(0x)?[[:xdigit:]]{12}\b';
> >>>
> >>> On 64-bit powerpc (ppc64/ppc64le) we'd want:
> >>>
> >>> +my $address = '\b(0x)?[89abcdef]00[[:xdigit:]]{13}\b';
> >>>
> >>>
>  +# Do not parse these files (absolute path).
>  +my @skip_parse_files_abs = ('/proc/kmsg',
>  +'/proc/kcore',
>  +'/proc/fs/ext4/sdb1/mb_groups',
>  +'/proc/1/fd/3',
>  +'/sys/kernel/debug/tracing/trace_pipe',
>  +'/sys/kernel/security/apparmor/revision');
> >>>
> >>> Can you add:
> >>>
> >>>   /sys/firmware/devicetree
> >>>
> >>> and/or /proc/device-tree (which is a symlink to the above).
> >>
> >> /proc/device-tree is a symlink to /sys/firmware/devicetree/base
> > 
> > Oh yep, forgot about the base part.
> > 
> >> /sys/firmware contains
> >>fdt  -- the flattened device tree that was passed to the
> >>kernel on boot
> >>devicetree/base/ -- the data that is currently in the live device tree.
> >>This live device tree is represented as directories
> >>and files beneath base/
> >>
> >> The information in fdt is directly available in the kernel source tree
> > 
> > On ARM that might be true, but not on powerpc.

Looks like we should be considering architecture specific lists for
files/directories to skip.

thanks,
Tobin.


Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

2017-11-12 Thread Frank Rowand
Hi Michael,

On 11/12/17 03:49, Michael Ellerman wrote:
> Hi Frank,
> 
> Frank Rowand  writes:
>> Hi Michael, Tobin,
>>
>> On 11/08/17 04:10, Michael Ellerman wrote:
>>> "Tobin C. Harding"  writes:
 Currently we are leaking addresses from the kernel to user space. This
 script is an attempt to find some of those leakages. Script parses
 `dmesg` output and /proc and /sys files for hex strings that look like
 kernel addresses.

 Only works for 64 bit kernels, the reason being that kernel addresses
 on 64 bit kernels have '' as the leading bit pattern making greping
 possible.
>>>
>>> That doesn't work super well on other architectures :D
>>>
>>> I don't speak perl but presumably you can check the arch somehow and
>>> customise the regex?
>>>
>>> ...
 +# Return _all_ non false positive addresses from $line.
 +sub extract_addresses
 +{
 +my ($line) = @_;
 +my $address = '\b(0x)?[[:xdigit:]]{12}\b';
>>>
>>> On 64-bit powerpc (ppc64/ppc64le) we'd want:
>>>
>>> +my $address = '\b(0x)?[89abcdef]00[[:xdigit:]]{13}\b';
>>>
>>>
 +# Do not parse these files (absolute path).
 +my @skip_parse_files_abs = ('/proc/kmsg',
 +  '/proc/kcore',
 +  '/proc/fs/ext4/sdb1/mb_groups',
 +  '/proc/1/fd/3',
 +  '/sys/kernel/debug/tracing/trace_pipe',
 +  '/sys/kernel/security/apparmor/revision');
>>>
>>> Can you add:
>>>
>>>   /sys/firmware/devicetree
>>>
>>> and/or /proc/device-tree (which is a symlink to the above).
>>
>> /proc/device-tree is a symlink to /sys/firmware/devicetree/base
> 
> Oh yep, forgot about the base part.
> 
>> /sys/firmware contains
>>fdt  -- the flattened device tree that was passed to the
>>kernel on boot
>>devicetree/base/ -- the data that is currently in the live device tree.
>>This live device tree is represented as directories
>>and files beneath base/
>>
>> The information in fdt is directly available in the kernel source tree
> 
> On ARM that might be true, but not on powerpc.
> 
> Remember FDT comes from DT which comes from OF - in which case the
> information is definitely not in the kernel source! :)
> 
> On our bare metal machines the device tree comes from skiboot
> (firmware), with some of the content provided by hostboot (other
> firmware), both of which are open source, so in theory most of the
> information is available in *some* source tree. But there's still
> information about runtime allocations etc. that is not available in the
> source anywhere.

Thanks for the additional information. 

Can you explain a little bit what "runtime allocations" are?  Are you
referring to the memory reservation block, the memory node(s) and the
chosen node?  Or other information?


Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

2017-11-12 Thread Michael Ellerman
Hi Frank,

Frank Rowand  writes:
> Hi Michael, Tobin,
>
> On 11/08/17 04:10, Michael Ellerman wrote:
>> "Tobin C. Harding"  writes:
>>> Currently we are leaking addresses from the kernel to user space. This
>>> script is an attempt to find some of those leakages. Script parses
>>> `dmesg` output and /proc and /sys files for hex strings that look like
>>> kernel addresses.
>>>
>>> Only works for 64 bit kernels, the reason being that kernel addresses
>>> on 64 bit kernels have '' as the leading bit pattern making greping
>>> possible.
>> 
>> That doesn't work super well on other architectures :D
>> 
>> I don't speak perl but presumably you can check the arch somehow and
>> customise the regex?
>> 
>> ...
>>> +# Return _all_ non false positive addresses from $line.
>>> +sub extract_addresses
>>> +{
>>> +my ($line) = @_;
>>> +my $address = '\b(0x)?[[:xdigit:]]{12}\b';
>> 
>> On 64-bit powerpc (ppc64/ppc64le) we'd want:
>> 
>> +my $address = '\b(0x)?[89abcdef]00[[:xdigit:]]{13}\b';
>> 
>> 
>>> +# Do not parse these files (absolute path).
>>> +my @skip_parse_files_abs = ('/proc/kmsg',
>>> +   '/proc/kcore',
>>> +   '/proc/fs/ext4/sdb1/mb_groups',
>>> +   '/proc/1/fd/3',
>>> +   '/sys/kernel/debug/tracing/trace_pipe',
>>> +   '/sys/kernel/security/apparmor/revision');
>> 
>> Can you add:
>> 
>>   /sys/firmware/devicetree
>> 
>> and/or /proc/device-tree (which is a symlink to the above).
>
> /proc/device-tree is a symlink to /sys/firmware/devicetree/base

Oh yep, forgot about the base part.

> /sys/firmware contains
>fdt  -- the flattened device tree that was passed to the
>kernel on boot
>devicetree/base/ -- the data that is currently in the live device tree.
>This live device tree is represented as directories
>and files beneath base/
>
> The information in fdt is directly available in the kernel source tree

On ARM that might be true, but not on powerpc.

Remember FDT comes from DT which comes from OF - in which case the
information is definitely not in the kernel source! :)

On our bare metal machines the device tree comes from skiboot
(firmware), with some of the content provided by hostboot (other
firmware), both of which are open source, so in theory most of the
information is available in *some* source tree. But there's still
information about runtime allocations etc. that is not available in the
source anywhere.

cheers


Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

2017-11-10 Thread Frank Rowand
Hi Michael, Tobin,

On 11/08/17 04:10, Michael Ellerman wrote:
> "Tobin C. Harding"  writes:
>> Currently we are leaking addresses from the kernel to user space. This
>> script is an attempt to find some of those leakages. Script parses
>> `dmesg` output and /proc and /sys files for hex strings that look like
>> kernel addresses.
>>
>> Only works for 64 bit kernels, the reason being that kernel addresses
>> on 64 bit kernels have '' as the leading bit pattern making greping
>> possible.
> 
> That doesn't work super well on other architectures :D
> 
> I don't speak perl but presumably you can check the arch somehow and
> customise the regex?
> 
> ...
>> +# Return _all_ non false positive addresses from $line.
>> +sub extract_addresses
>> +{
>> +my ($line) = @_;
>> +my $address = '\b(0x)?[[:xdigit:]]{12}\b';
> 
> On 64-bit powerpc (ppc64/ppc64le) we'd want:
> 
> +my $address = '\b(0x)?[89abcdef]00[[:xdigit:]]{13}\b';
> 
> 
>> +# Do not parse these files (absolute path).
>> +my @skip_parse_files_abs = ('/proc/kmsg',
>> +'/proc/kcore',
>> +'/proc/fs/ext4/sdb1/mb_groups',
>> +'/proc/1/fd/3',
>> +'/sys/kernel/debug/tracing/trace_pipe',
>> +'/sys/kernel/security/apparmor/revision');
> 
> Can you add:
> 
>   /sys/firmware/devicetree
> 
> and/or /proc/device-tree (which is a symlink to the above).

/proc/device-tree is a symlink to /sys/firmware/devicetree/base

/sys/firmware contains
   fdt  -- the flattened device tree that was passed to the
   kernel on boot
   devicetree/base/ -- the data that is currently in the live device tree.
   This live device tree is represented as directories
   and files beneath base/

The information in fdt is directly available in the kernel source tree
(possible exception: the bootloader may have modified the fdt, possibly
to add/modify the boot command line, add memory size).

The information in devicetree/base/ is directly available in the kernel
source tree for _most_ architectures, with the same possible exception
for the bootloader.  ppc64 may also modify this information dynamically
after the system is booted.  When overlay support is working, overlay
device trees will also be able to modify this information dynamically
(and again, this information will be directly available in the kernel
source tree).

Not having read the code in leaking_addresses.pl, trusting that the
comments are correct, it seems that /sys/firmware should be in
@skip_walk_dirs_abs instead of putting /sys/firmware/devicetree
in @skip_parse_files_abs.


> We should also start restricting access to that because it may have
> potentially interesting physical addresses in it, but that will break
> existing tools, so it will need to be opt-in and done over time.
> 
> cheers
> 



Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

2017-11-10 Thread kaiwan . billimoria
On Tue, 2017-11-07 at 21:32 +1100, Tobin C. Harding wrote:
> Currently we are leaking addresses from the kernel to user space.
> This
> script is an attempt to find some of those leakages. Script parses
> `dmesg` output and /proc and /sys files for hex strings that look
> like
> kernel addresses.
> 
> Only works for 64 bit kernels, the reason being that kernel addresses
> on 64 bit kernels have '' as the leading bit pattern making
> greping
> possible. On 32 kernels we don't have this luxury.

Tobin C. Harding  wrote:
>Only works for 64 bit kernels, the reason being that kernel addresses
>on 64 bit kernels have '' as the leading bit pattern making greping
>possible. On 32 kernels we don't have this luxury.

[RFC] leaking_addresses.pl - enhance it to work for 32-bit kernels as well

(Firstly, apologies if I've got the protocol horribly wrong- should this
be a new thread altogether?)

Ok so, I was interested in figuring - why not have this useful script work
for 32-bit kernel virtual addresses as well (and those systems by
extension).

The approach am considering, pl correct me if I'm way off:
on 32-bit, the kernel macro PAGE_OFFSET will give us the user-kernel split;
(alternatively, could also script up CONFIG_VMSPLIT_[n]G and figure the
split from there.)

For the time being, lets say we go with the "use PAGE_OFFSET" approach and
PAGE_OFFSET = 0xc000 , whch implies we have a 3:1 GB user:kernel split.
So any virtual addresses >= PAGE_OFFSET are kernel virtual addresses (i
know, untrue on some ARM-32 systems!).

As a very early and *far-from-perfect* start, I've enhanced Tobin's Perl
script to take into account 32-bit address space by passing the
parameter '--bit-size='.

The patch below does Not take into account (yet) stuff like:
 - exactly which files & dirs should be skipped on 32-bit (will it be
identical to 64-bit?; unsure..)
 - it currently hard-codes a global 'PAGE_OFFSET_32BIT=0xc000' , just
 so I can test quickly; must figure whether to query it or pass it;
 Suggestions?
 - the 'false positives'; again, what differs for 32-bit?
   (BTW, shouldn't the dmesg 'root=UUID=<...>' line be a false positive
& skipped?).

Also, I must point out that I'm a complete newbie to Perl :-) so, pl excuse
my highly inadequate perl-foo; I rely on you perl gurus out there to fix
and optimize :)

Yes, I've **Very Minimally** tested the patch in it's current form on:
a) a regular (Fedora 26) x86_64 desktop,
b) a (Debian 7) 32-bit kernel (VM) with PAGE_OFFSET=3 Gb
and it seems all right, considering...

Some sample output from test (b), if interested:
=
dmesg: [0.00] found SMP MP-table at [c00f1280] f1280
dmesg: [0.00] Base memory trampoline at [c009b000] 9b000 size 16384
dmesg: [0.00] ACPI: Local APIC address 0xfee0
dmesg: [0.00] free_area_init_node: node 0, pgdat c1418bc0, node_mem_map 
dfbfa200
dmesg: [0.00] ACPI: Local APIC address 0xfee0
dmesg: [0.00] ACPI: IOAPIC (id[0x00] address[0xfec0] gsi_base[0])
dmesg: [0.00] IOAPIC[0]: apic_id 0, version 17, address 0xfec0, GSI 
0-23
dmesg: [0.00] PERCPU: Embedded 14 pages/cpu @dfbe8000 s33344 r0 d24000 
u57344
dmesg: [0.00] fixmap  : 0xffd36000 - 0xf000   (2852 kB)
dmesg: [0.00] pkmap   : 0xffa0 - 0xffc0   (2048 kB)
dmesg: [0.00] vmalloc : 0xe07fb000 - 0xff9fe000   ( 498 MB)
dmesg: [0.00] lowmem  : 0xc000 - 0xdfffb000   ( 511 MB)
dmesg: [0.00]   .init : 0xc1421000 - 0xc148c000   ( 428 kB)

[...]

/proc/kallsyms: c10010e8 T _stext
/proc/kallsyms: c1002000 T hypercall_page
/proc/kallsyms: c1003000 t arch_local_save_flags
/proc/kallsyms: c1003007 t arch_local_irq_enable
/proc/kallsyms: c100300e T do_one_initcall

<< ... plenty more kallsyms of course (92.5% of the output to be precise!) ... 
>>

/proc/modules: loop 17803 0 - Live 0xe097c000
/proc/modules: crc32c_intel 12659 0 - Live 0xe096e000
/proc/modules: snd_pcm 53461 0 - Live 0xe09f5000
/proc/modules: snd_page_alloc 12867 1 snd_pcm, Live 0xe0957000
/proc/modules: snd_timer 22401 1 snd_pcm, Live 0xe093c000

[...]

/proc/modules: usb_common 12338 1 usbcore, Live 0xe086
/proc/timer_list:   .base:   dfbeb8b0
/proc/timer_list:  #0: , tick_sched_timer, S:01, 
hrtimer_start_range_ns, swapper/0/0

[...]

/proc/iomem:   f800-fbff : :00:02.0
/proc/iomem:   fc00-fcff : :00:02.0
/proc/iomem:   fd00-fd03 : :00:03.0

[...]

/proc/11422/syscall: 7 0x 0xbf814618 0xa 0xa 0x0 0x1 0xbf8145b8 
0xb7780428
/proc/11422/stack: [] kmap_atomic_prot+0x2f/0xe0
/proc/11422/stack: [] security_task_wait+0xc/0xd

[...]

/proc/bus/input/devices: B: KEY=4 200 3803078 f800d001 fedf ffef 
 fffe
/proc/1/net/ipv6_route:  00 
 00   
0001 000f 00200200   lo

[...]

/proc/2/net/unix: dce872c0: 

Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

2017-11-09 Thread Kaiwan N Billimoria
>
> Yes, profiling and tracing are similar. And you need to be root to run
> the recording anyway. Thus, as long as root user can read kallsyms,
> trace-cmd should be fine. As trace-cmd requires root access to do any
> ftrace tracing.
>
> -- Steve
Got it, thanks..


Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

2017-11-09 Thread Steven Rostedt
On Thu, 9 Nov 2017 10:24:32 +0530
Kaiwan N Billimoria  wrote:

> On Thu, Nov 9, 2017 at 10:13 AM, Kaiwan N Billimoria
>  wrote:
> >> But I don't know if there is anything else than the profiling code
> >> that _really_ wants access to /proc/kallsyms in user space as a
> >> regular user.  
> >  
> 
> Front-ends to ftrace, like trace-cmd?
> [from the trace-cmd git repo (Steve Rostedt, pl stand up, pl stand up :-)
> Documentation/trace-cmd-restore.1.txt :

Yes, profiling and tracing are similar. And you need to be root to run
the recording anyway. Thus, as long as root user can read kallsyms,
trace-cmd should be fine. As trace-cmd requires root access to do any
ftrace tracing.

-- Steve



Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

2017-11-08 Thread Kaiwan N Billimoria
On Thu, Nov 9, 2017 at 10:13 AM, Kaiwan N Billimoria
 wrote:
>> But I don't know if there is anything else than the profiling code
>> that _really_ wants access to /proc/kallsyms in user space as a
>> regular user.
>

Front-ends to ftrace, like trace-cmd?
[from the trace-cmd git repo (Steve Rostedt, pl stand up, pl stand up :-)
Documentation/trace-cmd-restore.1.txt :
...
*-k* kallsyms::
Used with *-c*, it overrides where to read the kallsyms file from.
By default, /proc/kallsyms is used. *-k* will override the file to
read the kallsyms from. This can be useful if the trace.dat file
to create is from another machine. Just copy the /proc/kallsyms
file locally, and use *-k* to point to that file.
...
]

> Am unsure about this, but kprobes? (/jprobes/kretprobes), and by
> extension, wrappers over this infra (like SystemTap)?
> I (hazily) recollect a script I once wrote (years back though) that
> collects kernel virtual addresses off of kallsyms for the purpose of
> passing them to a 'helper' kernel module that uses kprobes. I realize
> that 'modern' kprobes exposes APIs that just require the symbolic name
> & that they're anyway at kernel privilege... but the point is, a
> usermode script was picking up and passing the kernel addresses.
>
> Also, what about kernel addresses exposed via System.map?
> Oh, just checked, it's root rw only.. pl ignore.


Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

2017-11-08 Thread Kaiwan N Billimoria
> But I don't know if there is anything else than the profiling code
> that _really_ wants access to /proc/kallsyms in user space as a
> regular user.

Am unsure about this, but kprobes? (/jprobes/kretprobes), and by
extension, wrappers over this infra (like SystemTap)?
I (hazily) recollect a script I once wrote (years back though) that
collects kernel virtual addresses off of kallsyms for the purpose of
passing them to a 'helper' kernel module that uses kprobes. I realize
that 'modern' kprobes exposes APIs that just require the symbolic name
& that they're anyway at kernel privilege... but the point is, a
usermode script was picking up and passing the kernel addresses.

Also, what about kernel addresses exposed via System.map?
Oh, just checked, it's root rw only.. pl ignore.

> That said, that patch also fixes the /proc/kallsyms root check, in
> that now you can do:
>
> sudo head < /proc/kallsyms
>
> and it still shows all zeroes - because the file was *opened* as a
> normal user. That's how UNIX file access security works, and how it is
> fundamentally supposed to work (ie passing a file descriptor to a sui
> program doesn't magically make it gain privileges).

Indeed.


Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

2017-11-08 Thread Tobin C. Harding
On Thu, Nov 09, 2017 at 11:49:52AM +1100, Michael Ellerman wrote:
> "Tobin C. Harding"  writes:
> 
> > On Wed, Nov 08, 2017 at 11:10:56PM +1100, Michael Ellerman wrote:
> >> "Tobin C. Harding"  writes:
> > [snip]
> >
> > Hi Michael,
> >
> > I'm working an adding support for ppc64 to leaking_addresses.pl, I've
> > added the kernel address regular expression that you suggested.
> 
> Thanks!
> 
> > I'd like to add the false positive for vsyscall addresses. Excuse my
> > ignorance but does PowerPC use a constant address range for vsyscall like 
> > x86_64
> > does? The ppc64 machine I have access to does not output anything for
> >
> > $ cat /proc/PID/tasks/PID/smaps or
> > $ cat /proc/PID/tasks/PID/maps
> 
> No we only have the vdso style vsyscall, which is mapped at user
> addresses and is subject to ASLR, so you shouldn't need to worry about
> it.

Great. I'll add you to the CC list for the next spin. In line with my
aim of having the most confusing patches to follow the next version will
likely be

[PATCH 0/X v2] scripts/leaking_addresses: add summary report

thanks,
Tobin.


Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

2017-11-08 Thread Michael Ellerman
"Tobin C. Harding"  writes:

> On Wed, Nov 08, 2017 at 11:10:56PM +1100, Michael Ellerman wrote:
>> "Tobin C. Harding"  writes:
> [snip]
>
> Hi Michael,
>
> I'm working an adding support for ppc64 to leaking_addresses.pl, I've
> added the kernel address regular expression that you suggested.

Thanks!

> I'd like to add the false positive for vsyscall addresses. Excuse my
> ignorance but does PowerPC use a constant address range for vsyscall like 
> x86_64
> does? The ppc64 machine I have access to does not output anything for
>
>   $ cat /proc/PID/tasks/PID/smaps or
>   $ cat /proc/PID/tasks/PID/maps

No we only have the vdso style vsyscall, which is mapped at user
addresses and is subject to ASLR, so you shouldn't need to worry about
it.

cheers


Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

2017-11-08 Thread Tobin C. Harding
On Wed, Nov 08, 2017 at 11:10:56PM +1100, Michael Ellerman wrote:
> "Tobin C. Harding"  writes:
[snip]

Hi Michael,

I'm working an adding support for ppc64 to leaking_addresses.pl, I've
added the kernel address regular expression that you suggested. I'd like
to add the false positive for vsyscall addresses. Excuse my ignorance
but does PowerPC use a constant address range for vsyscall like x86_64
does? The ppc64 machine I have access to does not output anything for

$ cat /proc/PID/tasks/PID/smaps or
$ cat /proc/PID/tasks/PID/maps

thanks,
Tobin.


Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

2017-11-08 Thread Tobin C. Harding
On Wed, Nov 08, 2017 at 11:10:56PM +1100, Michael Ellerman wrote:
> "Tobin C. Harding"  writes:
> > Currently we are leaking addresses from the kernel to user space. This
> > script is an attempt to find some of those leakages. Script parses
> > `dmesg` output and /proc and /sys files for hex strings that look like
> > kernel addresses.
> >
> > Only works for 64 bit kernels, the reason being that kernel addresses
> > on 64 bit kernels have '' as the leading bit pattern making greping
> > possible.
> 
> That doesn't work super well on other architectures :D
> 
> I don't speak perl but presumably you can check the arch somehow and
> customise the regex?

I'm on it.

> ...
> > +# Return _all_ non false positive addresses from $line.
> > +sub extract_addresses
> > +{
> > +my ($line) = @_;
> > +my $address = '\b(0x)?[[:xdigit:]]{12}\b';
> 
> On 64-bit powerpc (ppc64/ppc64le) we'd want:
> 
> +my $address = '\b(0x)?[89abcdef]00[[:xdigit:]]{13}\b';

This is great! Thanks a million. This gives me the idea of getting in
contact with people who have access to other [64 bit] architectures and
getting the address format. I guess a dump of kallsyms from each
architecture would do the job nicely. 

> > +# Do not parse these files (absolute path).
> > +my @skip_parse_files_abs = ('/proc/kmsg',
> > +   '/proc/kcore',
> > +   '/proc/fs/ext4/sdb1/mb_groups',
> > +   '/proc/1/fd/3',
> > +   '/sys/kernel/debug/tracing/trace_pipe',
> > +   '/sys/kernel/security/apparmor/revision');
> 
> Can you add:
> 
>   /sys/firmware/devicetree
> 
> and/or /proc/device-tree (which is a symlink to the above).

Can do, thanks.

> We should also start restricting access to that because it may have
> potentially interesting physical addresses in it, but that will break
> existing tools, so it will need to be opt-in and done over time.

Seems like this is going to be a recurring theme if we try to stop leaks
using file permissions. I'm interested in how we would do this, assuming
it has to be a case by case fix but done many times.

thanks,
Tobin.


Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

2017-11-08 Thread Linus Torvalds
On Tue, Nov 7, 2017 at 4:59 PM, Linus Torvalds
 wrote:
>
> For example, maybe /proc/kallsyms could just default to not showing
> values to non-root users.
>
> Something like the attached TOTALLY UNTESTED patch. It's meant more as
> an RFC, not for application, but it's also meant to show how we can
> tailor the behavior for specific workflow issues.

It seems to work, except I got the condition wrong for
sysctl_perf_event_paranoid.

It should if

if (sysctl_perf_event_paranoid <= 1)
return 1;

rather than "<= 0", because '1' still means "allow kernel profiling"
(and the default value is "2").

But I don't know if there is anything else than the profiling code
that _really_ wants access to /proc/kallsyms in user space as a
regular user.

That said, that patch also fixes the /proc/kallsyms root check, in
that now you can do:

sudo head < /proc/kallsyms

and it still shows all zeroes - because the file was *opened* as a
normal user. That's how UNIX file access security works, and how it is
fundamentally supposed to work (ie passing a file descriptor to a sui
program doesn't magically make it gain privileges).

Anyway, I'm obviously not going to commit that patch now, but I'd be
happy to try it for the 4.15 merge window, to see if we can close
/proc/kallsyms without people screaming..

Linus


Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

2017-11-08 Thread Michael Ellerman
"Tobin C. Harding"  writes:
> Currently we are leaking addresses from the kernel to user space. This
> script is an attempt to find some of those leakages. Script parses
> `dmesg` output and /proc and /sys files for hex strings that look like
> kernel addresses.
>
> Only works for 64 bit kernels, the reason being that kernel addresses
> on 64 bit kernels have '' as the leading bit pattern making greping
> possible.

That doesn't work super well on other architectures :D

I don't speak perl but presumably you can check the arch somehow and
customise the regex?

...
> +# Return _all_ non false positive addresses from $line.
> +sub extract_addresses
> +{
> +my ($line) = @_;
> +my $address = '\b(0x)?[[:xdigit:]]{12}\b';

On 64-bit powerpc (ppc64/ppc64le) we'd want:

+my $address = '\b(0x)?[89abcdef]00[[:xdigit:]]{13}\b';


> +# Do not parse these files (absolute path).
> +my @skip_parse_files_abs = ('/proc/kmsg',
> + '/proc/kcore',
> + '/proc/fs/ext4/sdb1/mb_groups',
> + '/proc/1/fd/3',
> + '/sys/kernel/debug/tracing/trace_pipe',
> + '/sys/kernel/security/apparmor/revision');

Can you add:

  /sys/firmware/devicetree

and/or /proc/device-tree (which is a symlink to the above).

We should also start restricting access to that because it may have
potentially interesting physical addresses in it, but that will break
existing tools, so it will need to be opt-in and done over time.

cheers


Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

2017-11-07 Thread Tobin C. Harding
On Tue, Nov 07, 2017 at 03:36:06PM -0800, Laura Abbott wrote:
> On 11/07/2017 02:32 AM, Tobin C. Harding wrote:
> >Currently we are leaking addresses from the kernel to user space. This
> >script is an attempt to find some of those leakages. Script parses
> >`dmesg` output and /proc and /sys files for hex strings that look like
> >kernel addresses.
> >
> >Only works for 64 bit kernels, the reason being that kernel addresses
> >on 64 bit kernels have '' as the leading bit pattern making greping
> >possible. On 32 kernels we don't have this luxury.
> >
> >Scripts is _slightly_ smarter than a straight grep, we check for false
> >positives (all 0's or all 1's, and vsyscall start/finish addresses).
> >
> >Output is saved to file to expedite repeated formatting/viewing of
> >output.
> >
> >Signed-off-by: Tobin C. Harding 
> >---
> >
> >This version outputs a report instead of the raw results by default. 
> >Designing
> >this proved to be non-trivial, the reason being that it is not immediately 
> >clear
> >what constitutes a duplicate entry (similar message, address range, same
> >file?). Also, the aim of the report is to assist users _not_ missing correct
> >results; limiting the output is inherently a trade off between noise and
> >correct, clear results.
> >
> >Without testing on various real kernels its not clear that this reporting is 
> >any
> >good, my test cases were a bit contrived. Your usage may vary.
> >
> >It would be super helpful to get some comments from people running this with
> >different set ups.
> >
> 
> Running on a stock Fedora kernel with gnome generates a 139M file.
> I'll admit that Fedora is pretty generous in what it enables.
> Trimmed down to omit some redundancies in various processes
> by only printing off of the last file in the path
> 
> /proc/kallsyms
> /proc/modules
> /proc/timer_list
> /proc/1244/stack
> /proc/4041/status
> /proc/bus/input/devices <--- Probably a false positive
> /proc/1/net/hci
> /proc/1/net/tcp
> /proc/1/net/udp
> /proc/1/net/bnep
> /proc/1/net/raw6
> /proc/1/net/tcp6
> /proc/1/net/udp6
> /proc/1/net/unix
> /proc/1/net/l2cap
> /proc/1/net/packet
> /proc/1/net/rfcomm
> /proc/1/net/netlink
> /sys/module/snd_compress/sections/.note.gnu.build-id
> /sys/module/snd_compress/sections/.exit.text
> /sys/module/snd_compress/sections/__mcount_loc
> /sys/module/snd_compress/sections/__ksymtab_strings
> /sys/module/snd_compress/sections/__ksymtab_gpl
> /sys/module/snd_compress/sections/.init.text
> /sys/module/snd_compress/sections/.gnu.linkonce.this_module
> /sys/module/snd_compress/sections/__jump_table
> /sys/module/snd_compress/sections/.strtab
> /sys/module/snd_compress/sections/.bss
> /sys/module/snd_compress/sections/.rodata.str1.1
> /sys/module/snd_compress/sections/__bug_table
> /sys/module/snd_compress/sections/__verbose
> /sys/module/snd_compress/sections/.rodata.str1.8
> /sys/module/snd_compress/sections/.text
> /sys/module/snd_compress/sections/.data
> /sys/module/snd_compress/sections/.symtab
> /sys/module/snd_compress/sections/.rodata
> /sys/module/iwlmvm/sections/.altinstr_replacement
> /sys/module/iwlmvm/sections/.altinstructions
> /sys/module/iwlmvm/sections/.data.unlikely
> /sys/module/iwlmvm/sections/__param
> /sys/module/iwlmvm/sections/.smp_locks
> /sys/module/snd_hda_intel/sections/__tracepoints_ptrs
> /sys/module/snd_hda_intel/sections/__tracepoints
> /sys/module/snd_hda_intel/sections/__tracepoints_strings
> /sys/module/snd_hda_intel/sections/_ftrace_events
> /sys/module/snd_hda_intel/sections/.ref.data
> /sys/module/iwlwifi/sections/.parainstructions
> /sys/module/iwlwifi/sections/__ksymtab
> /sys/module/uvcvideo/sections/.fixup
> /sys/module/uvcvideo/sections/.text.unlikely
> /sys/module/uvcvideo/sections/__ex_table
> /sys/module/intel_powerclamp/sections/.init.rodata
> /sys/module/mac80211/sections/.data..read_mostly
> /sys/module/nfnetlink/sections/.init.data
> /sys/module/ghash_clmulni_intel/sections/.rodata.cst16.bswap_mask
> /sys/module/videodev/sections/_ftrace_eval_map
> /sys/module/kvm_intel/sections/.data..ro_after_init
> /sys/module/kvm_intel/sections/.altinstr_aux
> /sys/module/crct10dif_pclmul/sections/.rodata.cst16.SHUF_MASK
> /sys/module/crct10dif_pclmul/sections/.rodata.cst16.mask1
> /sys/module/crct10dif_pclmul/sections/.rodata.cst32.pshufb_shf_table
> /sys/module/crct10dif_pclmul/sections/.rodata.cst16.mask2
> /sys/module/nf_conntrack/sections/.data..cacheline_aligned
> /sys/firmware/efi/runtime-map/5/virt_addr
> /sys/devices/platform/i8042/serio0/input/input3/uevent
> /sys/devices/platform/i8042/serio0/input/input3/capabilities/key

thanks for running the script. Is there any chance you could email me
the complete output please? The next patch includes a flag to do
this. You can wait until that lands if it is easier for you.

thanks,
Tobin.


Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

2017-11-07 Thread Linus Torvalds
On Tue, Nov 7, 2017 at 3:36 PM, Laura Abbott  wrote:
>
> I'd probably put /proc/kallsyms and /proc/modules on the omit list
> since those are designed to leak addresses to userspace.

Well, they are indeed designed to leak addresses, but not a lot of
people should care.

So I think we could tighten them up.

For example, maybe /proc/kallsyms could just default to not showing
values to non-root users.

We *did* originally try to use "kptr_restrict" with a default value of
1, it's just that it was never fixable on a case-by-case basis as
people started saying "that breaks my flow, because xyz".

But if we do it for one file at a time, we probably *can* try to fix complaints.

Something like the attached TOTALLY UNTESTED patch. It's meant more as
an RFC, not for application, but it's also meant to show how we can
tailor the behavior for specific workflow issues.

So take that "kallsyms_for_perf()" thing as an example of how we can
say "hey, if you already have access to kernel profiling anyway,
there's no point in hiding kallsyms".

And there may be other similar things we can do.

The situation with /proc/modules should be similar. Using
kptr_restrict was a big hammer and might have broken something
unrelated, but did anybody actually care about the particular case of
/proc/modules not showing the module address to normal users? probably
not. "lsmod" certainly doesn't care, and that's what people really
want.

Both /proc/kallsyms and /proc/modules _used_ to be really important
for oops reporting, but that was long ago when the kernel didn't
report symbol information of its own. So we have historical reasons
for people to be able to read those files, but those are mainly things
that aren't relevant (or even possible) on modern kernels anyway.

So I don'r think we should omit /proc/kallsyms and /proc/modules - we
should just fix them.

The attached patch may not be good enough as is, but maybe something
_like_ it will work well enough that people won't care?

(And do note the "TOTALLY UNTESTED". It seems to compile. But maybe I
got some test exactly the wrong way around and it doesn't actually
_work_. Caveat testor).

 Linus
 kernel/kallsyms.c | 49 +++--
 1 file changed, 47 insertions(+), 2 deletions(-)

diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 127e7cfafa55..5b1299c1e4b0 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -480,6 +480,7 @@ struct kallsym_iter {
char name[KSYM_NAME_LEN];
char module_name[MODULE_NAME_LEN];
int exported;
+   int show_value;
 };
 
 static int get_ksymbol_mod(struct kallsym_iter *iter)
@@ -580,14 +581,23 @@ static void s_stop(struct seq_file *m, void *p)
 {
 }
 
+#ifndef CONFIG_64BIT
+# define KALLSYM_FMT "%08lx"
+#else
+# define KALLSYM_FMT "%016lx"
+#endif
+
 static int s_show(struct seq_file *m, void *p)
 {
+   unsigned long value;
struct kallsym_iter *iter = m->private;
 
/* Some debugging symbols have no name.  Ignore them. */
if (!iter->name[0])
return 0;
 
+   value = iter->show_value ? iter->value : 0;
+
if (iter->module_name[0]) {
char type;
 
@@ -597,10 +607,10 @@ static int s_show(struct seq_file *m, void *p)
 */
type = iter->exported ? toupper(iter->type) :
tolower(iter->type);
-   seq_printf(m, "%pK %c %s\t[%s]\n", (void *)iter->value,
+   seq_printf(m, KALLSYM_FMT " %c %s\t[%s]\n", value,
   type, iter->name, iter->module_name);
} else
-   seq_printf(m, "%pK %c %s\n", (void *)iter->value,
+   seq_printf(m, KALLSYM_FMT " %c %s\n", value,
   iter->type, iter->name);
return 0;
 }
@@ -612,6 +622,40 @@ static const struct seq_operations kallsyms_op = {
.show = s_show
 };
 
+static inline int kallsyms_for_perf(void)
+{
+#ifdef CONFIG_PERF_EVENTS
+   extern int sysctl_perf_event_paranoid;
+   if (sysctl_perf_event_paranoid <= 0)
+   return 1;
+#endif
+   return 0;
+}
+
+/*
+ * We show kallsyms information even to normal users if we've enabled
+ * kernel profiling and are explicitly not paranoid (so kptr_restrict
+ * is clear, and sysctl_perf_event_paranoid isn't set).
+ *
+ * Otherwise, require CAP_SYSLOG (assuming kptr_restrict isn't set to
+ * block even that).
+ */
+static int kallsyms_show_value(void)
+{
+   switch (kptr_restrict) {
+   case 0:
+   if (kallsyms_for_perf())
+   return 1;
+   /* fallthrough */
+   case 1:
+   if (has_capability_noaudit(current, CAP_SYSLOG))
+   return 1;
+   /* fallthrough */
+   default:
+   return 0;
+   }
+}
+
 static int kallsyms_open(struct inode *inode, struct file *file)
 {
/*
@@ -625,6 +669,7 @@ static int kallsyms_open(struct i

Re: [kernel-hardening] [PATCH v4] scripts: add leaking_addresses.pl

2017-11-07 Thread Laura Abbott

On 11/07/2017 02:32 AM, Tobin C. Harding wrote:

Currently we are leaking addresses from the kernel to user space. This
script is an attempt to find some of those leakages. Script parses
`dmesg` output and /proc and /sys files for hex strings that look like
kernel addresses.

Only works for 64 bit kernels, the reason being that kernel addresses
on 64 bit kernels have '' as the leading bit pattern making greping
possible. On 32 kernels we don't have this luxury.

Scripts is _slightly_ smarter than a straight grep, we check for false
positives (all 0's or all 1's, and vsyscall start/finish addresses).

Output is saved to file to expedite repeated formatting/viewing of
output.

Signed-off-by: Tobin C. Harding 
---

This version outputs a report instead of the raw results by default. Designing
this proved to be non-trivial, the reason being that it is not immediately clear
what constitutes a duplicate entry (similar message, address range, same
file?). Also, the aim of the report is to assist users _not_ missing correct
results; limiting the output is inherently a trade off between noise and
correct, clear results.

Without testing on various real kernels its not clear that this reporting is any
good, my test cases were a bit contrived. Your usage may vary.

It would be super helpful to get some comments from people running this with
different set ups.



Running on a stock Fedora kernel with gnome generates a 139M file.
I'll admit that Fedora is pretty generous in what it enables.
Trimmed down to omit some redundancies in various processes
by only printing off of the last file in the path

/proc/kallsyms
/proc/modules
/proc/timer_list
/proc/1244/stack
/proc/4041/status
/proc/bus/input/devices <--- Probably a false positive
/proc/1/net/hci
/proc/1/net/tcp
/proc/1/net/udp
/proc/1/net/bnep
/proc/1/net/raw6
/proc/1/net/tcp6
/proc/1/net/udp6
/proc/1/net/unix
/proc/1/net/l2cap
/proc/1/net/packet
/proc/1/net/rfcomm
/proc/1/net/netlink
/sys/module/snd_compress/sections/.note.gnu.build-id
/sys/module/snd_compress/sections/.exit.text
/sys/module/snd_compress/sections/__mcount_loc
/sys/module/snd_compress/sections/__ksymtab_strings
/sys/module/snd_compress/sections/__ksymtab_gpl
/sys/module/snd_compress/sections/.init.text
/sys/module/snd_compress/sections/.gnu.linkonce.this_module
/sys/module/snd_compress/sections/__jump_table
/sys/module/snd_compress/sections/.strtab
/sys/module/snd_compress/sections/.bss
/sys/module/snd_compress/sections/.rodata.str1.1
/sys/module/snd_compress/sections/__bug_table
/sys/module/snd_compress/sections/__verbose
/sys/module/snd_compress/sections/.rodata.str1.8
/sys/module/snd_compress/sections/.text
/sys/module/snd_compress/sections/.data
/sys/module/snd_compress/sections/.symtab
/sys/module/snd_compress/sections/.rodata
/sys/module/iwlmvm/sections/.altinstr_replacement
/sys/module/iwlmvm/sections/.altinstructions
/sys/module/iwlmvm/sections/.data.unlikely
/sys/module/iwlmvm/sections/__param
/sys/module/iwlmvm/sections/.smp_locks
/sys/module/snd_hda_intel/sections/__tracepoints_ptrs
/sys/module/snd_hda_intel/sections/__tracepoints
/sys/module/snd_hda_intel/sections/__tracepoints_strings
/sys/module/snd_hda_intel/sections/_ftrace_events
/sys/module/snd_hda_intel/sections/.ref.data
/sys/module/iwlwifi/sections/.parainstructions
/sys/module/iwlwifi/sections/__ksymtab
/sys/module/uvcvideo/sections/.fixup
/sys/module/uvcvideo/sections/.text.unlikely
/sys/module/uvcvideo/sections/__ex_table
/sys/module/intel_powerclamp/sections/.init.rodata
/sys/module/mac80211/sections/.data..read_mostly
/sys/module/nfnetlink/sections/.init.data
/sys/module/ghash_clmulni_intel/sections/.rodata.cst16.bswap_mask
/sys/module/videodev/sections/_ftrace_eval_map
/sys/module/kvm_intel/sections/.data..ro_after_init
/sys/module/kvm_intel/sections/.altinstr_aux
/sys/module/crct10dif_pclmul/sections/.rodata.cst16.SHUF_MASK
/sys/module/crct10dif_pclmul/sections/.rodata.cst16.mask1
/sys/module/crct10dif_pclmul/sections/.rodata.cst32.pshufb_shf_table
/sys/module/crct10dif_pclmul/sections/.rodata.cst16.mask2
/sys/module/nf_conntrack/sections/.data..cacheline_aligned
/sys/firmware/efi/runtime-map/5/virt_addr
/sys/devices/platform/i8042/serio0/input/input3/uevent
/sys/devices/platform/i8042/serio0/input/input3/capabilities/key

I'd probably put /proc/kallsyms and /proc/modules on the omit list
since those are designed to leak addresses to userspace. The
modules in sysfs might be harder to lockdown.

Thanks,
Laura


Please feel free to say 'try harder Tobin, this reporting is shit'.

Thanks, appreciate your time,
Tobin.

v4:
  - Add `scan` and `format` sub-commands.
  - Output report by default.
  - Add command line option to send scan results (to me).

v3:
  - Iterate matches to check for results instead of matching input line against
false positives i.e catch lines that contain results as well as false
positives.

v2:
  - Add regex's to prevent false positives.
  - Clean up white space.

  MAINTAINERS