Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752488AbdLGBY0 (ORCPT ); Wed, 6 Dec 2017 20:24:26 -0500 Received: from mailout4.samsung.com ([203.254.224.34]:49761 "EHLO mailout4.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752447AbdLGBYX (ORCPT ); Wed, 6 Dec 2017 20:24:23 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 mailout4.samsung.com 20171207012421epoutp04a11ebdf5f23a66aaf5af501043f63fc7~93qejcvQ-2845728457epoutp04R X-AuditID: b6c32a45-e7bff700000010f7-98-5a28984416f0 MIME-version: 1.0 Content-transfer-encoding: 8BIT Content-type: text/plain; charset="UTF-8" Message-id: <5A289845.5040301@samsung.com> Date: Thu, 07 Dec 2017 10:24:21 +0900 From: Chanwoo Choi Organization: Samsung Electronics User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 To: "Gustavo A. R. Silva" , MyungJoo Ham , Kyungmin Park Cc: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] PM / devfreq: Fix potential NULL pointer dereference in governor_store In-reply-to: <20171206202015.GA15636@embeddedor.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrCKsWRmVeSWpSXmKPExsWy7bCmma7rDI0ogxXyFiuX7GezONv0ht3i 8q45bBafe48wWtxuXMHmwOqx7qCqR9+WVYwenzfJBTBHpdpkpCampBYppOYl56dk5qXbKnkH xzvHm5oZGOoaWlqYKynkJeam2iq5+AToumXmAC1UUihLzCkFCgUkFhcr6dvZFOWXlqQqZOQX l9gqRRsaGukZGpjrGRkZ6ZkYx1oZmQKVJKRmTLp1ga3gNXfFlb9aDYyXOLsYOTkkBEwkFuz9 ydLFyMUhJLCDUWL1o39QzndGiTk3fjHBVJ2f8YUJIrGBUeJI7yywBK+AoMSPyfeAOjg4mAXk JY5cygYJMwtoSmzdvZ4dov4eo8SqXatYIeq1JN7dWMMCYrMIqEp83HQJbA4bUHz/ixtsIDa/ gKLE1R+PGUFsUYEIiZ3zv4ENEhHoZJRYemw3O8QGK4nXH7vBbGGBWImu8/PAhnICXfryyn5G iKs3sElM7oX600Xi5qdzLBC2sMSr41vYIWxpiWerNjKCLJAQaGeUaN87jxnCmcIoce76Paj/ jSWeLexigtjMJ9Fx+C87yMsSArwSHW1CECUeEt1dW9kgwo4SG98UgISFQGZu3xA3gVFuFlJ4 zUKE1yyk8FrAyLyKUSy1oDg3PbXYqMBQrzgxt7g0L10vOT93EyM4hWm57mCccc7nEKMAB6MS D6/FL/UoIdbEsuLK3EOMEhzMSiK8v/s1ooR4UxIrq1KL8uOLSnNSiw8xmgKDeyKzlGhyPjC9 5pXEG5pYGpiYmRmZm1kA05c4b/22axFCAumJJanZqakFqUUwfUwcnFINjM+sFHL5hOSWhm6/ eUqnk831UNKyDU+/e74UX301aG12p1PwY0GpHfJ/zmw+/l114hXTg1tC3uvI/Lx25dJk20Ni bFNvrFBYfnfBHWktlrh/q1klzZS7Zx9cKF93/Kvl8WsfUz3b3/asvJQ9Q/DhTAHlQ4Zmu9Ki OJwbt9XtKrrmLLrlycNbN9iUWIozEg21mIuKEwEQSTn5dwMAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrJLMWRmVeSWpSXmKPExsVy+t9jQV2XGRpRBr8vGFmsXLKfzeJs0xt2 i8u75rBZfO49wmhxu3EFmwOrx7qDqh59W1YxenzeJBfAHMVlk5Kak1mWWqRvl8CVMenWBbaC 19wVV/5qNTBe4uxi5OSQEDCROD/jC1MXIxeHkMA6RomXE/+wgiR4BQQlfky+x9LFyMHBLCAv ceRSNkiYWUBdYtK8RcwQ9Q8YJbbf6GSHqNeSeHdjDQuIzSKgKvFx0yUmEJsNKL7/xQ02EJtf QFHi6o/HjCAzRQUiJLpPVILMERHoZpSY0bqHDWKBlcTrj91gM4UFYiW6zs9jgVjWySjR/X4+ 2AJOoKtfXtnPOIFRYBaSW2ch3DoLya0LGJlXMUqmFhTnpucWGxUY5aWW6xUn5haX5qXrJefn bmIEBvC2w1r9OxgfL4k/xCjAwajEw/vin3qUEGtiWXFl7iFGCQ5mJRHe3/0aUUK8KYmVValF +fFFpTmpxYcYpTlYlMR5+fOPRQoJpCeWpGanphakFsFkmTg4pRoYufZNNjkqvVA3/3XmiqS/ l47vNrSM3WNsJsFq4FqgyjS78cL/crkdu4/tYrA4Jza56yBDSN4lvf/XW8SmF9wryom8vWA7 z+W9C4zyt7/dmbj+170lq8tuii1jWnRcWcHtof7Ta8WGx1btXFy3SnS5hHiA0YMko+0du7tM k+NlLSf0GOsEOioGK7EUZyQaajEXFScCABmKjDJcAgAA X-CMS-MailID: 20171207012420epcas2p47d7f84a41010ee4ca86dc7af430d265e X-Msg-Generator: CA CMS-TYPE: 102P DLP-Filter: Pass X-CFilter-Loop: Reflected X-CMS-RootMailID: 20171206202028epcas2p44920d43cbde12943ed978b1e6d192ba5 X-RootMTR: 20171206202028epcas2p44920d43cbde12943ed978b1e6d192ba5 References: <20171206202015.GA15636@embeddedor.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1473 Lines: 42 On 2017년 12월 07일 05:20, Gustavo A. R. Silva wrote: > df->governor is being dereferenced before it is null checked, > hence there is a potential null pointer dereference. > > Notice that df->governor is being null checked at line 1004: > if (df->governor) {, which implies it might be null. > > Fix this by null checking df->governor before dereferencing it. > > Addresses-Coverity-ID: 1401988 ("Dereference before null check") > Fixes: bcf23c79c4e4 ("PM / devfreq: Fix available_governor sysfs") > Signed-off-by: Gustavo A. R. Silva > --- > drivers/devfreq/devfreq.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > index 78fb496..14fe76b 100644 > --- a/drivers/devfreq/devfreq.c > +++ b/drivers/devfreq/devfreq.c > @@ -996,7 +996,8 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr, > if (df->governor == governor) { > ret = 0; > goto out; > - } else if (df->governor->immutable || governor->immutable) { > + } else if ((df->governor && df->governor->immutable) || > + governor->immutable) { > ret = -EINVAL; > goto out; > } > Actually, df->governor would be never NULL because devfreq_add_device() initializes the ->governor always. But, governor_store() doesn't know it. So, looks good to me. Reviewed-by: Chanwoo Choi -- Best Regards, Chanwoo Choi Samsung Electronics