From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 94378C47095 for ; Wed, 9 Jun 2021 09:32:49 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 5DA2961285 for ; Wed, 9 Jun 2021 09:32:49 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5DA2961285 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=oracle.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Q/R+UhJujk5KHr6edqXhRakVAQrZAabFD+u2zbaV9hk=; b=hcOq+vTsSQTwvN d34pLWBOvuZppXAutbe4Ky3T9TyDr4NoUIBDfVyl3LUxroPUMHeKgFajJLQrhVhxlSI6lU+xKmpPh wXonNiuIR4YHdOisDIL+yVnHo+ybqD6QhyLZnlDfv0uCE8NWO5OrjiAcyXFO0itGvfnfMekr7EeMr xgQRc1uy5JjnQ3DpMMusayGaXtUDN/5RRy3PwhWwGTzy7ME1nUHpAm5Tg91RRiEoI9PjYrFAV2nzM S+IQUydgh/nf74fYPJK6ihv5RZUTwv+7xUIUK1IwlfuNZvPc72eACWDh+jR65U7H19SJkKGMt54C4 ssXVBqWcmmeK/DNFmp2w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1lquY6-00Cgpy-0A; Wed, 09 Jun 2021 09:30:59 +0000 Received: from mx0a-00069f02.pphosted.com ([205.220.165.32]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1lqu4t-00CX6K-O8 for linux-arm-kernel@lists.infradead.org; Wed, 09 Jun 2021 09:00:49 +0000 Received: from pps.filterd (m0246617.ppops.net [127.0.0.1]) by mx0b-00069f02.pphosted.com (8.16.0.43/8.16.0.43) with SMTP id 1598tngo021330; Wed, 9 Jun 2021 08:59:55 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=date : from : to : cc : subject : message-id : references : mime-version : content-type : in-reply-to; s=corp-2020-01-29; bh=yxZ33z/ij2fUQhSTHogXfHEeZX6RxjggVDZ8OIpu7LI=; b=ZWVYoKxI73lvE2I6snU7+AToxrGwT/OKuq7+RNkbXJ0bzKokdLsW/mdbgGbOHsKmpeF8 6hUa0q08UdMDYm9zI3K24Cs3M9Ra/HzZofgzlxS1Gc9kvWaNF4E/6sJXKj/rPDl0wh9x 8islMhjpdhTDbqztkUsn70N8Rj1ZZ0T/0eAF4P8CfmJXAoj7Vwp2i/KW/2N9fxn8wsoX jnLUgzWwUkWBgB8zdiCEpo3FfFE/OqAwdv95ribMFa9iBWSwf12LnrdRoLYW7Lpmhzhj hbfhl1fY5eqByeg/jrtHVv6mQK8BIxMnp13lvGhBYJiRRhAxBn/51+hWoinj9aKDCKAk sw== Received: from oracle.com (userp3020.oracle.com [156.151.31.79]) by mx0b-00069f02.pphosted.com with ESMTP id 391g4g8w2k-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 09 Jun 2021 08:59:55 +0000 Received: from userp3020.oracle.com (userp3020.oracle.com [127.0.0.1]) by pps.podrdrct (8.16.0.36/8.16.0.36) with SMTP id 1598v8fs066583; Wed, 9 Jun 2021 08:59:54 GMT Received: from pps.reinject (localhost [127.0.0.1]) by userp3020.oracle.com with ESMTP id 390k1rrkg4-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 09 Jun 2021 08:59:54 +0000 Received: from userp3020.oracle.com (userp3020.oracle.com [127.0.0.1]) by pps.reinject (8.16.0.36/8.16.0.36) with SMTP id 1598xreW077970; Wed, 9 Jun 2021 08:59:53 GMT Received: from aserv0122.oracle.com (aserv0122.oracle.com [141.146.126.236]) by userp3020.oracle.com with ESMTP id 390k1rrkfu-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 09 Jun 2021 08:59:53 +0000 Received: from abhmp0001.oracle.com (abhmp0001.oracle.com [141.146.116.7]) by aserv0122.oracle.com (8.14.4/8.14.4) with ESMTP id 1598xmAk001991; Wed, 9 Jun 2021 08:59:48 GMT Received: from kadam (/41.212.42.34) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Wed, 09 Jun 2021 08:59:47 +0000 Date: Wed, 9 Jun 2021 11:59:38 +0300 From: Dan Carpenter To: William Breathitt Gray Cc: jic23@kernel.org, linux-stm32@st-md-mailman.stormreply.com, kernel@pengutronix.de, a.fatoum@pengutronix.de, kamel.bouhara@bootlin.com, gwendal@chromium.org, alexandre.belloni@bootlin.com, david@lechnology.com, linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, syednwaris@gmail.com, patrick.havelange@essensium.com, fabrice.gasnier@st.com, mcoquelin.stm32@gmail.com, alexandre.torgue@st.com, o.rempel@pengutronix.de, jarkko.nikula@linux.intel.com Subject: Re: [PATCH v11 26/33] counter: Add character device interface Message-ID: <20210609085938.GM10983@kadam> References: <2b9526ab7f2de91bb867cbd3b12552c77c00b655.1623201082.git.vilhelm.gray@gmail.com> <20210609080708.GL10983@kadam> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.4 (2018-02-28) X-Proofpoint-GUID: 4I8RCZTTf-Fxzs_HCqW4XL5igtPktEGw X-Proofpoint-ORIG-GUID: 4I8RCZTTf-Fxzs_HCqW4XL5igtPktEGw X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210609_020047_860923_EA0F67A8 X-CRM114-Status: GOOD ( 27.33 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Jun 09, 2021 at 05:28:10PM +0900, William Breathitt Gray wrote: > On Wed, Jun 09, 2021 at 11:07:08AM +0300, Dan Carpenter wrote: > > On Wed, Jun 09, 2021 at 10:31:29AM +0900, William Breathitt Gray wrote: > > > +static int counter_set_event_node(struct counter_device *const counter, > > > + struct counter_watch *const watch, > > > + const struct counter_comp_node *const cfg) > > > +{ > > > + struct counter_event_node *event_node; > > > + struct counter_comp_node *comp_node; > > > + > > > > The caller should be holding the counter->events_list_lock lock but it's > > not. > > Hi Dan, > > The counter_set_event_node() function doesn't access or modify > counter->events_list (it works on counter->next_events_list) so holding > the counter->events_list_lock here isn't necessary. > There needs to be some sort of locking or this function can race with itself. (Two threads add the same event at exactly the same time). It looks like it can also race with counter_disable_events() leading to a use after free. > > > + /* Search for event in the list */ > > > + list_for_each_entry(event_node, &counter->next_events_list, l) > > > + if (event_node->event == watch->event && > > > + event_node->channel == watch->channel) > > > + break; > > > + > > > + /* If event is not already in the list */ > > > + if (&event_node->l == &counter->next_events_list) { > > > + /* Allocate new event node */ > > > + event_node = kmalloc(sizeof(*event_node), GFP_ATOMIC); Btw, say we decided that we can add/remove events locklessly, then these GFP_ATOMICs can be changed to GFP_KERNEL. > > > + if (!event_node) > > > + return -ENOMEM; > > > + > > > + /* Configure event node and add to the list */ > > > + event_node->event = watch->event; > > > + event_node->channel = watch->channel; > > > + INIT_LIST_HEAD(&event_node->comp_list); > > > + list_add(&event_node->l, &counter->next_events_list); > > > + } > > > + > > > + /* Check if component watch has already been set before */ > > > + list_for_each_entry(comp_node, &event_node->comp_list, l) > > > + if (comp_node->parent == cfg->parent && > > > + comp_node->comp.count_u8_read == cfg->comp.count_u8_read) > > > + return -EINVAL; > > > + > > > + /* Allocate component node */ > > > + comp_node = kmalloc(sizeof(*comp_node), GFP_ATOMIC); ^^^^^^^^^^ > > > + if (!comp_node) { > > > + /* Free event node if no one else is watching */ > > > + if (list_empty(&event_node->comp_list)) { > > > + list_del(&event_node->l); > > > + kfree(event_node); > > > + } > > > + return -ENOMEM; > > > + } > > > + *comp_node = *cfg; > > > + > > > + /* Add component node to event node */ > > > + list_add_tail(&comp_node->l, &event_node->comp_list); > > > + > > > + return 0; > > > +} regards, dan carpenter _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel