Received: by 2002:a25:6193:0:0:0:0:0 with SMTP id v141csp2697125ybb; Mon, 30 Mar 2020 11:06:51 -0700 (PDT) X-Google-Smtp-Source: ADFU+vuvz8AnedAzGrXxAIAoAgfnqsT0QephW8B0a/Ygvj/xuPAXvmvLkgqo+7TWUOeLdHcWSUME X-Received: by 2002:a05:6808:4e:: with SMTP id v14mr451038oic.70.1585591611785; Mon, 30 Mar 2020 11:06:51 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1585591611; cv=none; d=google.com; s=arc-20160816; b=OI37iLntIQzNkgEYrKvcY49Vq3EsIzK4MKUTH+78I7T3/And9s7f7qUsRmocDg/D70 jaJ2l0C6pHoCE2RNqD7oz/K4Gx6+n+v22B3QRhDXwl9vx3NKUWxTsZfISEDXWhMmzGlT sGbqOEXF1P10LbqnhNgERxV3oB5UeqZHvatXLctSmUQLIjj9MV05HzPu6iVJEgpFm0i3 EWs+/0jXl2BCzZ/kOxJXeeGc01YN5M55+kU/IsxVQjaMLrgHbCFBY3KJs5qhAXgvkzJB I06SElkWmwfpEAQQ2BxaxZIgi5BIIiJ6S7PWKsCcFk0DK2myCIP6f0HuSkNJN0v56rZi 6dFA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-transfer-encoding:content-disposition:mime-version :references:message-id:subject:cc:to:date:from:dkim-signature; bh=3yni0Qcm8n05TZCBj3hKRu8NlwE4SJ7yEQomCEGLDwk=; b=Z0KPMAr6oWaov/bM4crJJ0iBgRyicN9pUbJK3ODbfTSbV/mFKaJlVGLF6PRDqtFJfI hn5Zn3VD6rb+z6CAsOfM4Nrfx4W/4dQaDUUPw/U7Yg4/K9t0ztqTAW2rc54P8YUZH0PL 5iylG2SJLPhjYgZDW/TLpCSxMUAruLOZkuqvif7VSCmsKOf47rn3/VLRwPqvtC0oz3rM 1LhuMr6cUt7tZU0ig7qiWJNntGiWuSsYv7DG8fFaEK8Glq8u6ePrZYqSXhKKBmLZctCg QaydWgmvC9dT9aXJR4XqwYHxpTGBH0xzdwFhaLisHmcKPkQKiOop3ncYbFt6m6LoNu7S LVWw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=pxhpA2L1; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id m22si5655941otn.81.2020.03.30.11.06.36; Mon, 30 Mar 2020 11:06:51 -0700 (PDT) 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; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=pxhpA2L1; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727899AbgC3RU6 (ORCPT + 99 others); Mon, 30 Mar 2020 13:20:58 -0400 Received: from mail-pl1-f182.google.com ([209.85.214.182]:44189 "EHLO mail-pl1-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726497AbgC3RU5 (ORCPT ); Mon, 30 Mar 2020 13:20:57 -0400 Received: by mail-pl1-f182.google.com with SMTP id h11so6974762plr.11; Mon, 30 Mar 2020 10:20:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:date:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=3yni0Qcm8n05TZCBj3hKRu8NlwE4SJ7yEQomCEGLDwk=; b=pxhpA2L10LsUBSIcO30K/eICyF7HWIxFaI9tUeDTf63aY3YXDhdwI7iGYk3Z8c2FoV 3Cz7NtUP+cgrgbTtT4DjY4cdHgQmrkZ3coJQZlj/V0D7xqz7MqzG1fUYe99AW71DI8zr 9raQIzS5GzOIAajfVqp8v+a8C7jylqolpKsmr8DaMnA4a2Temw4YOJ8dGKtC0HTvBGjC N8NfMHzcA2iCnb8Xoynpa/rwuMGdgp+FpgOAgJiVRQOdxEHjDTa6z8CJuUifB41/nMQ/ AUgrGiXPeqoWBDiVavraPnqcBX8k//OPulwAYNAk0gXUK0nmI2RfMEBirWr5VhWMmeAr vavA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:date:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=3yni0Qcm8n05TZCBj3hKRu8NlwE4SJ7yEQomCEGLDwk=; b=tQj9o9Rzi3V2S//VJMET1ABcSQHajL/ccccN3q019TH5tsKxiQCxRY152/vqb3iWZz iF9Vt9UAx12f4KKXJk6mNfIVQvkiHYTiir5J0Ml13moOz7/tC3vDUl42gIwWseeh6/Pp Kjkw7qdTB0lhCPcmCQJWY7HQBeGssIYPtaLGozHeJZpWDP+xAdnnFz3WBedbOK4pBPHz VAK/41bhgdfsGchV5RvJfT7HOYTUlhWaMhvQvLWzkYWuoeCg7DpYOg059rVSMRv9Ke7r +QWaESC6cpzbH73qVQqJ0/zM+SsOlsusJxAIzOxEogG7KL2RMjDnhNihLY4f7BFIS5iS J7Ng== X-Gm-Message-State: AGi0PuZVI33MQRykpLLQWoB25T9NT0l++sbcO3ddhXWeD+lOzBBaIMjm piDIPg36WmIITKl5LWmXoQ== X-Received: by 2002:a17:902:76c6:: with SMTP id j6mr4216084plt.223.1585588856352; Mon, 30 Mar 2020 10:20:56 -0700 (PDT) Received: from madhuparna-HP-Notebook ([2402:3a80:d3b:3d6b:7942:93fd:fd15:96f0]) by smtp.gmail.com with ESMTPSA id s61sm96571pjd.33.2020.03.30.10.20.51 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 30 Mar 2020 10:20:55 -0700 (PDT) From: Madhuparna Bhowmik X-Google-Original-From: Madhuparna Bhowmik Date: Mon, 30 Mar 2020 22:50:48 +0530 To: Alan Stern Cc: madhuparnabhowmik10@gmail.com, gregkh@linuxfoundation.org, hariprasad.kelam@gmail.com, colin.king@canonical.com, tony.olech@elandigitalsystems.com, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, andrianov@ispras.ru Subject: Re: Possible data-race related bug in u132_hcd module. Message-ID: <20200330172048.GA12976@madhuparna-HP-Notebook> References: <20200330115243.11107-1-madhuparnabhowmik10@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.9.4 (2018-02-28) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Mar 30, 2020 at 12:03:31PM -0400, Alan Stern wrote: > On Mon, 30 Mar 2020 madhuparnabhowmik10@gmail.com wrote: > > > Hi, > > > > This bug is found by? Linux Driver Verification project (linuxtesting.org). > > > > The bug is related to the parallel execution of u132_probe() function > > and?u132_hcd_exit() function in u132_hcd.c. In case the module is > > unloaded when the probe function is executing there can be data race > > as the mutex lock?u132_module_lock is not used properly.? > > Normally drivers do not have to worry about races between their probe > and exit routines. The exit routine should unregister the driver from > its bus subsystem, and unregistration is supposed to wait until all > probe and remove functions have finished executing. > > > i) Usage of mutex lock only when writing into the u132_exiting > > variable in?u132_hcd_exit(). The lock is not used when this variable > > is read in?u132_probe(). > > I'm not familiar with u132_hcd, but the probe routine shouldn't need to > use and "exiting" variable at all. > Even I am not sure why it should use this variable to check, that's why I thought of asking in the mailing list. If the maintainers agree that we can remove this variable I can send a patch doing it. This variable is not used for any other purpose in the module, so removing it shouldn't be a problem. > > > > Moreover, this variable does not serve its?purpose, as even if > > locking?is used while the u132_exiting variable is read in probe(), > > the function may still miss that exit function is executing if it > > acquires the mutex before exit() function does. > > > > How to fix this? > > Are you certain there really is a problem? > If the variable u132_exiting is really used for the purpose of checking if the module is exiting then there might be a problem. If at all it is assumed that exit never races with probe function and it is always called after probe finishes or something, then this variable is not even required. And suppose there is possibility of a race condition then only holding the mutex in exit but not probe does not make sense. > > ii) Usage of mutex while adding entries in?u132_static_list in probe > > function but not in exit function while unregistering. > > This should be easy to fix by holding the mutex in the exit function as well. > > Why does the driver need a static list? > I do not know much about this module so I cannot answer this. From the point of synchronization, this list is initialized in init and then only used in probe and exit function. And lock is only held in probe, if at all it is assumed that exit cannot race with probe, there is no need to use the mutex. > > There can be other synchronization problems related to the usage of > > u132_module_lock in this module, I have only spotted these so far. > > You should look at other drivers for comparison. They don't have to > face this kind of problem. u132_hcd should be similar to them. > Yes, I checked out a few others and I could not see any usage of such variable to check if module is exiting. Thank you, Madhuparna > Alan Stern > >