Bug 1327388

Summary: Fix use-after-free in st_queue_handler
Product: Red Hat Enterprise Linux 7 Reporter: Frank Sorenson <fsorenso>
Component: autofsAssignee: Ian Kent <ikent>
Status: CLOSED ERRATA QA Contact: xiaoli feng <xifeng>
Severity: medium Docs Contact:
Priority: medium    
Version: 7.2CC: eguan, xifeng
Target Milestone: rcKeywords: Patch, TestCaseProvided
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: autofs-5.0.7-56.el7 Doc Type: Bug Fix
Doc Text:
Cause: In function st_queue_handler() the task structure may be referenced after being freed. Consequence: This can occasionally lead to a segmentation fault. Fix: Move the line which references this task structure to a location before it is freed. Result: No segmentation fault can occur due to this reference.
Story Points: ---
Clone Of: Environment:
Last Closed: 2016-11-04 07:44:05 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
patch to fix the use-after-free
none
Patch - fix use-after-free in st_queue_handler() none

Description Frank Sorenson 2016-04-15 04:01:14 UTC
Description of problem:

In st_queue_handler, the 'task' may be referenced after being freed.


Version-Release number of selected component (if applicable):

kernel-3.10.0-327.10.1.el7.x86_64
autofs-5.0.7-54.el7.x86_64


How reproducible:

see below


Steps to Reproduce:

# echo "/autofs_segv	/etc/auto.segv0" > /etc/auto.master
# echo "subdir	-fstype=autofs	file:/etc/auto.segv1" > /etc/auto.segv0
# > /etc/auto.segv1

# systemctl restart autofs.service

(run under ElectricFence to get segfault when the freed memory is accessed)
# ef /usr/sbin/automount -t 20 -f

(in another terminal)
# ls /autofs_segv/subdir

(wait about 5 seconds)


Actual results:

segfault


Expected results:

automount continues running


Additional info:

here, 'next' gets a pointer in the 'task->pending' list, then 'task' is freed, but the 'next->pending' list may still include the 'task'.  The 'list_del_init(&next->pending)' will then attempt to write to the address:

	/* Next task */
	next = list_entry((&task->pending)->next,
		struct state_queue, pending);

	list_del(&task->list);
	free(task);

	list_del_init(&next->pending);  <<<<<<<<<<<<


97	static __inline__ void list_del_init(struct list_head *entry)
98	{
99		__list_del(entry->prev, entry->next); <<<<<<<<<
100		INIT_LIST_HEAD(entry); 


76	static __inline__ void __list_del(struct list_head * prev,
77					  struct list_head * next)
78	{
79		next->prev = prev;  <<<<<<<<<<<< crash while setting 'next->prev'
80		prev->next = next;


#0  0x00007fbc191ed89d in __list_del (prev=0x7fbc14f13fd8, next=0x7fbc14f13fd8) at ../include/list.h:79
#1  0x00007fbc191ed8fc in list_del_init (entry=0x7fbc15ea0fd8) at ../include/list.h:99
#2  0x00007fbc191f0592 in st_queue_handler (arg=0x0) at state.c:1187
#3  0x00007fbc18b91dc5 in start_thread (arg=0x7fbc190a3700) at pthread_create.c:308
#4  0x00007fbc179f821d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:113

in frame 2:
(gdb) p task
$4 = (struct state_queue *) 0x7fbc14f13fc0

(gdb) p &task->pending
$6 = (struct list_head *) 0x7fbc14f13fd8


(gdb) p next
$7 = (struct state_queue *) 0x7fbc15ea0fc0

(gdb) p &next->pending
$11 = (struct list_head *) 0x7fbc15ea0fd8


(gdb) p (&next->pending)->prev
$12 = (struct list_head *) 0x7fbc14f13fd8

(gdb) p (&next->pending)->next
$15 = (struct list_head *) 0x7fbc14f13fd8

so we're trying to write back to the '&task->pending' after freeing task

Comment 1 Frank Sorenson 2016-04-15 04:09:58 UTC
Created attachment 1147456 [details]
patch to fix the use-after-free

Comment 3 Ian Kent 2016-04-15 07:15:36 UTC
(In reply to Frank Sorenson from comment #0)
> Description of problem:
> 
> In st_queue_handler, the 'task' may be referenced after being freed.
> 
> 
> Version-Release number of selected component (if applicable):
> 
> kernel-3.10.0-327.10.1.el7.x86_64
> autofs-5.0.7-54.el7.x86_64
> 
> 
> How reproducible:
> 
> see below
> 
> 
> Steps to Reproduce:
> 
> # echo "/autofs_segv	/etc/auto.segv0" > /etc/auto.master
> # echo "subdir	-fstype=autofs	file:/etc/auto.segv1" > /etc/auto.segv0
> # > /etc/auto.segv1
> 
> # systemctl restart autofs.service
> 
> (run under ElectricFence to get segfault when the freed memory is accessed)
> # ef /usr/sbin/automount -t 20 -f
> 
> (in another terminal)
> # ls /autofs_segv/subdir
> 
> (wait about 5 seconds)
> 
> 
> Actual results:
> 
> segfault
> 
> 
> Expected results:
> 
> automount continues running
> 
> 
> Additional info:
> 
> here, 'next' gets a pointer in the 'task->pending' list, then 'task' is
> freed, but the 'next->pending' list may still include the 'task'.  The
> 'list_del_init(&next->pending)' will then attempt to write to the address:

OK, it's been a while since I looked at this code.

I'll have a look.

AFAICR these two lists should be distinct so freeing something
on one should not affect the other.

I'll need to work out how the senario your describing works, not
a big problem but will take some effort.

Ian

Comment 4 Ian Kent 2016-04-15 07:22:53 UTC
(In reply to Frank Sorenson from comment #0)
> Description of problem:
> 
> In st_queue_handler, the 'task' may be referenced after being freed.

snip ...

> 
> here, 'next' gets a pointer in the 'task->pending' list, then 'task' is
> freed, but the 'next->pending' list may still include the 'task'.  The
> 'list_del_init(&next->pending)' will then attempt to write to the address:
> 
> 	/* Next task */
> 	next = list_entry((&task->pending)->next,
> 		struct state_queue, pending);
> 
> 	list_del(&task->list);
> 	free(task);
> 
> 	list_del_init(&next->pending);  <<<<<<<<<<<<
> 
> 
> 97	static __inline__ void list_del_init(struct list_head *entry)
> 98	{
> 99		__list_del(entry->prev, entry->next); <<<<<<<<<
> 100		INIT_LIST_HEAD(entry); 
> 
> 
> 76	static __inline__ void __list_del(struct list_head * prev,
> 77					  struct list_head * next)
> 78	{
> 79		next->prev = prev;  <<<<<<<<<<<< crash while setting 'next->prev'
> 80		prev->next = next;

Oh right, enough said.

Ian

Comment 5 Ian Kent 2016-05-17 06:29:57 UTC
Created attachment 1158184 [details]
Patch - fix use-after-free in st_queue_handler()

Comment 6 Ian Kent 2016-05-17 06:31:47 UTC
Comment on attachment 1147456 [details]
patch to fix the use-after-free

Some minor changes were made to the name of the patch and layout of the description of Franks' original patch.

Comment 10 errata-xmlrpc 2016-11-04 07:44:05 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHBA-2016-2508.html