Bug 926169

Summary: stack value overwrite when using struct rand_pool_info
Product: [openSUSE] openSUSE Distribution Reporter: Thomas Blume <thomas.blume>
Component: DevelopmentAssignee: Thomas Blume <thomas.blume>
Status: RESOLVED FIXED QA Contact: E-mail List <qa-bugs>
Severity: Normal    
Priority: P5 - None CC: hare, meissner, systemd-maintainers, thomas.blume
Version: 13.2   
Target Milestone: ---   
Hardware: Other   
OS: Other   
Whiteboard:
Found By: --- Services Priority:
Business Priority: Blocker: ---
Marketing QA Status: --- IT Deployment: ---
Attachments: random-seed-accidental-stack-corruption.patch
0001-random-seed-accidental-stack-corruption.patch
use-rndaddentropy-ioctl-to-load-random-seed.patch

Description Thomas Blume 2015-04-07 09:13:20 UTC
Found on a systemd build with disabled gcc optimizations.
/usr/lib/systemd/systemd-random-seed segfaults when freeing a buffer.
Reason is that the stack value gets overwritten:

-->--
> 70            buf = malloc(buf_size);
> (gdb) n
> 71            if (!buf) {
> (gdb) p buf
> $1 = (void *) 0x40e010
> (gdb) p &buf
> $2 = (void **) 0x7fffffffe1f8
> (gdb) watch *(void**) 0x7fffffffe1f8
> Hardware watchpoint 2: *(void**) 0x7fffffffe1f8
> (gdb) continue
> Continuing.
> Hardware watchpoint 2: *(void**) 0x7fffffffe1f8
> 
> Old value = (void *) 0x40e010
> New value = (void *) 0x554d2f03
> main (argc=2, argv=0x7fffffffe318) at src/random-seed/random-seed.c:142
> 142                                    r = ioctl(random_fd,
> RNDADDENTROPY, &entropy);
> --<--
> 
> The culprit seems to be one row before the ioctl:
> 
>                                 entropy.buf[0] = ((__u32*)buf)[0];
> 


--<--
include/linux/random.h:

struct rand_pool_info {
        int     entropy_count;
        int     buf_size;
        __u32   buf[0];
};

        struct rand_pool_info entropy = {
                    .entropy_count = entropy_count,
                    .buf_size = buf_size,
        };
        entropy.buf[0] = ((__u32*)buf)[0];


which will overwrite random values on the stack.
-->--
Comment 1 Hannes Reinecke 2015-04-07 09:26:57 UTC
This also affects SLES12.
Comment 2 Hannes Reinecke 2015-04-07 09:32:25 UTC
Created attachment 630158 [details]
random-seed-accidental-stack-corruption.patch

random-seed: Accidental stack corruption.
Comment 3 Hannes Reinecke 2015-04-07 09:33:15 UTC
Should be fixed by the above patch. Please test.
Comment 4 Hannes Reinecke 2015-04-07 09:57:12 UTC
This is caused by a SUSE-specific patch (Use RNDADDENTROPY ioctl to load random seed, bug#892096).
So no need to send this patch upstream.
Comment 5 Thomas Blume 2015-04-07 14:44:26 UTC
Created attachment 630231 [details]
0001-random-seed-accidental-stack-corruption.patch

Had to modify the patch a little in order to convince gcc to compile it.
It seems to work: 

-->--
(gdb) 
142	                                entropy->buf[0] = ((__u32*)buf)[0];
2: &entropy->buf[0] = (__u32 *) 0x7fffffffe208
1: entropy->buf[0] = 0
(gdb) 
143	                                r = ioctl(random_fd, RNDADDENTROPY, entropy);
2: &entropy->buf[0] = (__u32 *) 0x7fffffffe208
1: entropy->buf[0] = 3422532695
--<--

and the segfault is gone now.
but buf doesn't show a proper value anymore:

-->--
(gdb) print ((__u32*)buf)[0]
value has been optimized out
--<--

Strange, because I thought I disabled all optimization via -O0 to gcc.

Someone should better check my changes.
Hannes, can you confirm that this patch is ok?
Comment 6 Hannes Reinecke 2015-04-08 06:30:00 UTC
That's perfectly okay.

'buf' is a temporary variable, which has been eliminated by gcc internally during compilation. Apparently gcc is allowed to do this even without -O.
Comment 7 Marcus Meissner 2015-04-08 07:09:22 UTC
This code still looks wrong.


You want to feed in just 4 bytes?
How large is buf_size?

You probably will need to do something like: 

entropy = malloc(sizeof(struct rand_pool_info)+buf_size);
entropy->entropy_count = entropy_count;
entropy->buf_size = buf_size;
memcpy(&entropy->buf,buf,buf_size);
ioctl(random_fd, RNDADDENTROPY, entropy);

Otherwise you are just adding more or less uninitialized stack data.
Comment 8 Hannes Reinecke 2015-04-08 07:13:24 UTC
I was hoping to avoid having to allocate the rand_pool_info. But yeah, you are right. I'll be updating the patch.
Comment 9 Dr. Werner Fink 2015-04-14 12:02:48 UTC
Created attachment 630992 [details]
use-rndaddentropy-ioctl-to-load-random-seed.patch

This is a changed version of the original use-rndaddentropy-ioctl-to-load-random-seed.patch as with this a pointer on struct rand_pool_info as well as the buffer is allocated in one step.
Comment 10 Thomas Blume 2015-04-15 13:41:00 UTC
(In reply to Werner Fink from comment #9)
> Created attachment 630992 [details]
> use-rndaddentropy-ioctl-to-load-random-seed.patch
> 
> This is a changed version of the original
> use-rndaddentropy-ioctl-to-load-random-seed.patch as with this a pointer on
> struct rand_pool_info as well as the buffer is allocated in one step.

Confirmed that the crash is gone with this patch.
Comment 13 Thomas Blume 2015-07-13 12:28:53 UTC
This is fixed.
Comment 14 Swamp Workflow Management 2015-07-30 14:09:59 UTC
SUSE-RU-2015:1318-1: An update that has 15 recommended fixes can now be installed.

Category: recommended (moderate)
Bug References: 906900,909358,919095,920195,921831,921898,921920,926169,927457,928265,931388,932284,933365,933512,934077
CVE References: 
Sources used:
SUSE Linux Enterprise Software Development Kit 12 (src):    systemd-210-68.2
SUSE Linux Enterprise Server 12 (src):    systemd-210-68.2
SUSE Linux Enterprise Desktop 12 (src):    systemd-210-68.2
Comment 15 Bernhard Wiedemann 2015-08-11 12:00:08 UTC
This is an autogenerated message for OBS integration:
This bug (926169) was mentioned in
https://build.opensuse.org/request/show/321835 42 / systemd
Comment 16 Swamp Workflow Management 2015-10-02 10:13:16 UTC
openSUSE-RU-2015:1669-1: An update that has 28 recommended fixes can now be installed.

Category: recommended (moderate)
Bug References: 900558,904214,906900,909358,912334,913517,916420,918118,920195,921831,921898,926169,927457,928265,931388,932284,933365,933512,933521,933533,934077,937512,937900,938908,939571,940264,941576,944132
CVE References: 
Sources used:
openSUSE 13.2 (src):    systemd-210-25.19.1, systemd-mini-210-25.19.1
Comment 17 Swamp Workflow Management 2016-02-03 14:47:39 UTC
openSUSE-RU-2016:0320-1: An update that has 146 recommended fixes can now be installed.

Category: recommended (moderate)
Bug References: 737690,742774,750845,818044,838475,841544,849870,852015,852021,852232,853293,854884,856389,856392,856858,857204,858864,859072,859365,860574,860937,861316,861489,863217,864745,864904,865834,866732,866933,867128,867663,867664,867840,868019,868230,868439,868931,869142,869603,872929,873432,873444,874665,875502,876587,876694,877021,877674,878525,880438,880732,881125,881559,881942,882393,882714,883565,884271,884403,885232,885288,886211,886599,886852,888178,888215,888612,889297,889357,890977,892096,892162,892300,893797,895087,896664,897799,897801,897803,898233,898240,898432,900558,901481,902240,902901,903009,903963,904214,904517,904828,905550,906709,906900,907318,907393,908476,909358,910643,911347,912030,912334,913517,916420,918118,919095,920195,921831,921898,921920,926169,927250,927457,928265,931388,932284,933365,933512,933521,933533,934077,934901,937512,937900,938908,939571,940264,941576,944132,944799,945282,947212,948458,948555,948705,949574,949683,949739,950510,951265,951663,953241,954336,954781,955635,961576
CVE References: 
Sources used:
openSUSE 13.1 (src):    systemd-210-40.1, systemd-mini-210-40.1