Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 18 Jan 2018 11:38:46 -0800
From:   Dmitry Torokhov <dmitry.torokhov@gmail.com>
To:     Nick Desaulniers <nick.desaulniers@gmail.com>
Cc:     Andrey Ryabinin <aryabinin@virtuozzo.com>,
        Alexander Potapenko <glider@google.com>,
        Dmitry Vyukov <dvyukov@google.com>, kasan-dev@googlegroups.com,
        usacrc@yahoo.com, stable@vger.kernel.org,
        Benjamin Tissoires <benjamin.tissoires@redhat.com>,
        Nick Dyer <nick@shmanahar.org>,
        Andrew Duggan <aduggan@synaptics.com>,
        Christophe JAILLET <christophe.jaillet@wanadoo.fr>,
        Guenter Roeck <linux@roeck-us.net>,
        linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Input: synaptics-rmi4 - prevent UAF reported by KASAN
Message-ID: <20180118193846.bwotik7ngarpienm@dtor-ws>
References: <1516170303-3963-1-git-send-email-nick.desaulniers@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1516170303-3963-1-git-send-email-nick.desaulniers@gmail.com>
User-Agent: NeoMutt/20170609 (1.8.3)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Jan 16, 2018 at 10:25:01PM -0800, Nick Desaulniers wrote:
> KASAN found a UAF due to dangling pointer. As the report below says,
> rmi_f11_attention() accesses drvdata->attn_data.data, which was freed in
> rmi_irq_fn.
> 
> [  311.424062] BUG: KASAN: use-after-free in
> rmi_f11_attention+0x526/0x5e0 [rmi_core]
> [  311.424067] Read of size 27 at addr ffff88041fd610db by task
> irq/131-i2c_hid/1162
> 
> [  311.424075] CPU: 0 PID: 1162 Comm: irq/131-i2c_hid Not tainted
> 4.15.0-rc8+ #2
> [  311.424076] Hardware name: Razer Blade Stealth/Razer, BIOS 6.05
> 01/26/2017
> [  311.424078] Call Trace:
> [  311.424086]  dump_stack+0xae/0x12d
> [  311.424090]  ? _atomic_dec_and_lock+0x103/0x103
> [  311.424094]  ? show_regs_print_info+0xa/0xa
> [  311.424099]  ? input_handle_event+0x10b/0x810
> [  311.424104]  print_address_description+0x65/0x229
> [  311.424108]  kasan_report.cold.5+0xa7/0x281
> [  311.424117]  rmi_f11_attention+0x526/0x5e0 [rmi_core]
> [  311.424123]  ? memcpy+0x1f/0x50
> [  311.424132]  ? rmi_f11_attention+0x526/0x5e0 [rmi_core]
> [  311.424143]  ? rmi_f11_probe+0x1e20/0x1e20 [rmi_core]
> [  311.424153]  ? rmi_process_interrupt_requests+0x220/0x2a0 [rmi_core]
> [  311.424163]  ? rmi_irq_fn+0x22c/0x270 [rmi_core]
> [  311.424173]  ? rmi_process_interrupt_requests+0x2a0/0x2a0 [rmi_core]
> [  311.424177]  ? free_irq+0xa0/0xa0
> [  311.424180]  ? irq_finalize_oneshot.part.39+0xeb/0x180
> [  311.424190]  ? rmi_process_interrupt_requests+0x2a0/0x2a0 [rmi_core]
> [  311.424193]  ? irq_thread_fn+0x3d/0x80
> [  311.424197]  ? irq_finalize_oneshot.part.39+0x180/0x180
> [  311.424200]  ? irq_thread+0x21d/0x290
> [  311.424203]  ? irq_thread_check_affinity+0x170/0x170
> [  311.424207]  ? remove_wait_queue+0x150/0x150
> [  311.424212]  ? kasan_unpoison_shadow+0x30/0x40
> [  311.424214]  ? __init_waitqueue_head+0xa0/0xd0
> [  311.424218]  ? task_non_contending.cold.55+0x18/0x18
> [  311.424221]  ? irq_forced_thread_fn+0xa0/0xa0
> [  311.424226]  ? irq_thread_check_affinity+0x170/0x170
> [  311.424230]  ? kthread+0x19e/0x1c0
> [  311.424233]  ? kthread_create_worker_on_cpu+0xc0/0xc0
> [  311.424237]  ? ret_from_fork+0x32/0x40
> 
> [  311.424244] Allocated by task 899:
> [  311.424249]  kasan_kmalloc+0xbf/0xe0
> [  311.424252]  __kmalloc_track_caller+0xd9/0x1f0
> [  311.424255]  kmemdup+0x17/0x40
> [  311.424264]  rmi_set_attn_data+0xa4/0x1b0 [rmi_core]
> [  311.424269]  rmi_raw_event+0x10b/0x1f0 [hid_rmi]
> [  311.424278]  hid_input_report+0x1a8/0x2c0 [hid]
> [  311.424283]  i2c_hid_irq+0x146/0x1d0 [i2c_hid]
> [  311.424286]  irq_thread_fn+0x3d/0x80
> [  311.424288]  irq_thread+0x21d/0x290
> [  311.424291]  kthread+0x19e/0x1c0
> [  311.424293]  ret_from_fork+0x32/0x40
> 
> [  311.424296] Freed by task 1162:
> [  311.424300]  kasan_slab_free+0x71/0xc0
> [  311.424303]  kfree+0x90/0x190
> [  311.424311]  rmi_irq_fn+0x1b2/0x270 [rmi_core]
> [  311.424319]  rmi_irq_fn+0x257/0x270 [rmi_core]
> [  311.424322]  irq_thread_fn+0x3d/0x80
> [  311.424324]  irq_thread+0x21d/0x290
> [  311.424327]  kthread+0x19e/0x1c0
> [  311.424330]  ret_from_fork+0x32/0x40
> 
> [  311.424334] The buggy address belongs to the object at
> ffff88041fd610c0
>                 which belongs to the cache kmalloc-64 of size 64
> [  311.424340] The buggy address is located 27 bytes inside of
>                 64-byte region [ffff88041fd610c0, ffff88041fd61100)
> [  311.424344] The buggy address belongs to the page:
> [  311.424348] page:ffffea00107f5840 count:1 mapcount:0 mapping:
>    (null) index:0x0
> [  311.424353] flags: 0x17ffffc0000100(slab)
> [  311.424358] raw: 0017ffffc0000100 0000000000000000 0000000000000000
> 00000001802a002a
> [  311.424363] raw: dead000000000100 dead000000000200 ffff8804228036c0
> 0000000000000000
> [  311.424366] page dumped because: kasan: bad access detected
> 
> [  311.424369] Memory state around the buggy address:
> [  311.424373]  ffff88041fd60f80: 00 00 00 00 00 00 00 00 00 00 00 00
> 00 00 00 00
> [  311.424377]  ffff88041fd61000: fb fb fb fb fb fb fb fb fc fc fc fc
> fb fb fb fb
> [  311.424381] >ffff88041fd61080: fb fb fb fb fc fc fc fc fb fb fb fb
> fb fb fb fb
> [  311.424384]                                                     ^
> [  311.424387]  ffff88041fd61100: fc fc fc fc fb fb fb fb fb fb fb fb
> fc fc fc fc
> [  311.424391]  ffff88041fd61180: fb fb fb fb fb fb fb fb fc fc fc fc
> fb fb fb fb
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com>
> ---
>  drivers/input/rmi4/rmi_driver.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> index 4f2bb59..141ea22 100644
> --- a/drivers/input/rmi4/rmi_driver.c
> +++ b/drivers/input/rmi4/rmi_driver.c
> @@ -230,8 +230,10 @@ static irqreturn_t rmi_irq_fn(int irq, void *dev_id)
>  		rmi_dbg(RMI_DEBUG_CORE, &rmi_dev->dev,
>  			"Failed to process interrupt request: %d\n", ret);
>  
> -	if (count)
> +	if (count) {
>  		kfree(attn_data.data);
> +		attn_data.data = NULL;
> +	}
>  
>  	if (!kfifo_is_empty(&drvdata->attn_fifo))
>  		return rmi_irq_fn(irq, dev_id);

Ugh, this code needs reworking as we really do not want recursion in an
interrupt handler. Anyway, this will suffice for stable; applied, thank
you.

-- 
Dmitry