From patchwork Sun Jul 2 22:27:38 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dmitry Torokhov X-Patchwork-Id: 9821791 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id EA05360224 for ; Sun, 2 Jul 2017 22:27:48 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id CB088262AE for ; Sun, 2 Jul 2017 22:27:48 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id BE62027E63; Sun, 2 Jul 2017 22:27:48 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id DFB42262AE for ; Sun, 2 Jul 2017 22:27:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751973AbdGBW1n (ORCPT ); Sun, 2 Jul 2017 18:27:43 -0400 Received: from mail-pg0-f65.google.com ([74.125.83.65]:34600 "EHLO mail-pg0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751950AbdGBW1l (ORCPT ); Sun, 2 Jul 2017 18:27:41 -0400 Received: by mail-pg0-f65.google.com with SMTP id j186so21099287pge.1; Sun, 02 Jul 2017 15:27:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=zTmtjbAGaho21Zg8AGlpD7eBMeRQ9o3FSefZBLl8YCc=; b=mkmX+W3Ogkqt1jjfeB2mfgMA4BtMPibDbELk6BQpDM9nWVDkebt3UPiF5cSXdDBxPk 2p0/0XmGFvYv2EF2YjxPOIexazyI6BCXQ/+6xKm81XG7hoE8w55P+vOag17IerKNSIfn tlPKDHYw1o3JgJn0vs4XpZi+EW5WKQY+fGwpzvFk9BOQW3Ox8HOSzji5p6ISswcY2vHA t/EkCII5pQbVA5U7D5F0VEMQ7Dv8GXHOfygHmKf4F4IGcm9p/hfYsFYXBdbwK4A1s4fZ zW7jjU3ON3zFQ4AZzHjUPiN/SoGyOR+y34XXoHXgt+1xuZ8MQJ4P9AuAZv2xb6IlLdE9 YMNA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=zTmtjbAGaho21Zg8AGlpD7eBMeRQ9o3FSefZBLl8YCc=; b=O6GDmRaNych94kEHYBIAQK+dKbQ6N/QICIjm9dDkDWOPs0ydUEE1o9v8t2ruPPsChW 3DUmrnlfxAoEDMm6Mr1vHwb5eudWpTDhbkBW4/4qYmfukZC7sqgtT/j4lMO58bgb+/9A fpvoa9ZTbBHzrWbh3iIpvfte3d21Yb8h+LDKmEcTOy/9wfhPkkB1QQStb2ID1LlXozca L5eijB/ZnCeweUMILfioIUADjdqQq66Y//cBfMkqwENdSARxt25LdRIoUg8K4zsaVem2 B5BYg/iuzs8jhpyx6DX+1GqJ0KHZuxz2uFa6TYPjCiRSkInkO+mdUJyBwoHiEg1Y3/mY VRAw== X-Gm-Message-State: AIVw112NTOc8JILvJE9ZLzpVgRrMmCHpOBJ/sXQoYOv6BSrhduUOa/id qFbpGUfFxbDGqg== X-Received: by 10.98.69.219 with SMTP id n88mr7014147pfi.192.1499034460981; Sun, 02 Jul 2017 15:27:40 -0700 (PDT) Received: from dtor-ws ([2620:0:1000:1311:e54c:47d0:8df5:8555]) by smtp.gmail.com with ESMTPSA id a71sm32270926pfl.129.2017.07.02.15.27.39 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Sun, 02 Jul 2017 15:27:40 -0700 (PDT) Date: Sun, 2 Jul 2017 15:27:38 -0700 From: "dmitry.torokhov@gmail.com" To: "chenhong (N)" Cc: vojtech@suse.cz, "Jinjian (Ken)" , "Wulizhen (Pss)" , LKML , Linux Input Subject: Re: [PATCH] Input: i8042: add a check in i8042_interrupt Message-ID: <20170702222738.GA8574@dtor-ws> References: <7952EC96A33C0E41A611E13B3EB75D4224F70FA0@DGGEMA501-MBX.china.huawei.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <7952EC96A33C0E41A611E13B3EB75D4224F70FA0@DGGEMA501-MBX.china.huawei.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi, On Sat, Jun 24, 2017 at 09:38:48AM +0000, chenhong (N) wrote: > Description of problem: > > Encounterd BUG case: > serio: i8042 KBD port at 0x60,0x64 irq 1 > BUG: unable to handle kernel NULL pointer dereference at 0000000000000050 > IP: [] _spin_lock_irqsave+0x1f/0x40 > PGD 0 > Oops: 0002 [#1] SMP > last sysfs file: > CPU 0 > Modules linked in: > > Pid: 1, comm: swapper Not tainted 2.6.32-358.el6.x86_64 #1 QEMU Standard PC (i440FX + PIIX, 1996) > RIP: 0010:[] [] _spin_lock_irqsave+0x1f/0x40 > RSP: 0018:ffff880028203cc0 EFLAGS: 00010082 > RAX: 0000000000010000 RBX: 0000000000000000 RCX: 0000000000000000 > RDX: 0000000000000282 RSI: 0000000000000098 RDI: 0000000000000050 > RBP: ffff880028203cc0 R08: ffff88013e79c000 R09: ffff880028203ee0 > R10: 0000000000000298 R11: 0000000000000282 R12: 0000000000000050 > R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000098 > FS: 0000000000000000(0000) GS:ffff880028200000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0018 ES: 0018 CR0: 000000008005003b > CR2: 0000000000000050 CR3: 0000000001a85000 CR4: 00000000001407f0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 > Process swapper (pid: 1, threadinfo ffff88013e79c000, task ffff88013e79b500) > Stack: > ffff880028203d00 ffffffff813de186 ffffffffffffff02 0000000000000000 > 0000000000000000 0000000000000000 0000000000000000 0000000000000098 > ffff880028203d70 ffffffff813e0162 ffff880028203d20 ffffffff8103b8ac > Call Trace: > > [] serio_interrupt+0x36/0xa0 > [] i8042_interrupt+0x132/0x3a0 > [] ? kvm_clock_read+0x1c/0x20 > [] ? kvm_clock_get_cycles+0x9/0x10 > [] handle_IRQ_event+0x60/0x170 > [] ? kvm_guest_apic_eoi_write+0x44/0x50 > [] handle_edge_irq+0xde/0x180 > [] handle_irq+0x49/0xa0 > [] do_IRQ+0x6c/0xf0 > [] ret_from_intr+0x0/0x11 > [] ? __do_softirq+0x73/0x1e0 > [] ? hrtimer_interrupt+0x14b/0x260 > [] ? call_softirq+0x1c/0x30 > [] ? do_softirq+0x65/0xa0 > [] ? irq_exit+0x85/0x90 > [] ? smp_apic_timer_interrupt+0x70/0x9b > [] ? apic_timer_interrupt+0x13/0x20 > Version-Release number of selected component (if applicable): > > RELEASE: 2.6.32-358.el6.x86_64 (Red Hat Enterprise Linux 6.4) > VERSION: #1 SMP Thu Feb 21 02:37:52 EST 2013 > > The problem was also reproduced on Red Hat Enterprise Linux 7.3-x86_64(3.10.0-514.el7) > > Cause of the problem: > static irqreturn_t i8042_interrupt(int irq, void *dev_id) > { > ...... > serio = port->exists ? port->serio : NULL; > ...... > if (likely(port->exists && !filtered)) > serio_interrupt(serio, data, dfl); > ...... > } > > static int i8042_start(struct serio *serio) > { > struct i8042_port *port = serio->port_data; > port->exists = true; > mb(); > return 0; > } > > > i8042_probe > | > i8042_setup_kbd --> request_irq(i8042_interrupt) > | > i8042_register_ports --> serio_queue_event(SERIO_REGISTER_PORT) --> serio_handle_event --> serio_add_port --> i8042_start > > > i8042_start which set port->exists be true may be called during the i8042_interrupt function according to the analysis of initialization order. If port->exists is set to be true after the assignment of serio and before calling of serio_interrupt, a NULL pointer will be passed to serio_interrupt. The latest upstream kernel still has this problem. > Thank you for your report and the correct analysis of the problem. > > Solution for this problem > Add a check for serio to prevent NULL pointer. > > --- a/drivers/input/serio/i8042.c 2017-06-19 16:44:50.890078100 +0800 > +++ b/drivers/input/serio/i8042.c 2017-06-19 16:45:20.230022700 +0800 > @@ -524,7 +524,7 @@ i8042_interruptspin_unlock_irqrestore(&i8042_lock, flags); > - if (likely(port->exists && !filtered)) > + if (likely(port->exists && !filtered && serio)) I do not think we need to check port->exists here, just checking serio should be enough. Also, please check Documentation/SubmittingPatches when submitting patches in the future. I believe we want the version of the patch below. Thanks! diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c index c52da651269b..2f19bdbf64b5 100644 --- a/drivers/input/serio/i8042.c +++ b/drivers/input/serio/i8042.c @@ -436,8 +436,10 @@ static int i8042_start(struct serio *serio) { struct i8042_port *port = serio->port_data; + spin_lock_irq(&i8042_lock); port->exists = true; - mb(); + spin_unlock_irq(&i8042_lock); + return 0; } @@ -450,16 +452,10 @@ static void i8042_stop(struct serio *serio) { struct i8042_port *port = serio->port_data; + spin_lock_irq(&i8042_lock); port->exists = false; - - /* - * We synchronize with both AUX and KBD IRQs because there is - * a (very unlikely) chance that AUX IRQ is raised for KBD port - * and vice versa. - */ - synchronize_irq(I8042_AUX_IRQ); - synchronize_irq(I8042_KBD_IRQ); port->serio = NULL; + spin_unlock_irq(&i8042_lock); } /* @@ -576,7 +572,7 @@ static irqreturn_t i8042_interrupt(int irq, void *dev_id) spin_unlock_irqrestore(&i8042_lock, flags); - if (likely(port->exists && !filtered)) + if (likely(serio && !filtered)) serio_interrupt(serio, data, dfl); out: