Re: [PATCH 1/2] mmc: block: Preinitialize ret in mmc_blk_issue_drv_op()

Path	csiph.com!news.mixmin.net!aioe.org!bofh.it!news.nic.it!robomod
From	Geert Uytterhoeven <geert@linux-m68k.org>
Newsgroups	linux.kernel
Subject	Re: [PATCH 1/2] mmc: block: Preinitialize ret in mmc_blk_issue_drv_op()
Date	Wed, 05 Jul 2017 17:10:02 +0200
Message-ID	<tZU6e-7Wi-13@gated-at.bofh.it> (permalink)
References	<tZOtQ-4hV-1@gated-at.bofh.it> <tZRi3-62l-17@gated-at.bofh.it>
Dkim-Signature	v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc:content-transfer-encoding; bh=G0QviTpIcU41gtdN0Bs/bodEqXLfOYlFPzXtuuhG02A=; b=mwEAWWX0k7z6Co7JWgjrJ1XpHdwH2AdXB4Q7OeNeU4xgL7pLC4Idqghw98cTKgBUmw uyxjAzutNmGPVRdyUKYpeaJbtJPKpFJFe6MQpMVic08DKUo4753thKiZW9vlzqGNf113 uX4EF609/V7X9gCDVoQ28xdIS45eBpos25Urvq/cMbNsO8pbZIXlQLMn0jmlliafahGq lGtMYeyaDKmVzqVxi3GF1SGgXv7teX45TUUKGGUPWu2lKB6oA2MCniL64LMk8TQA10XF wiBlGoSEmm9J7wfec19Hpg5SMsgIpsq21vX5Rzd9xDA61/QKdrQpy1c4n3hT/yaCFQMa kzRg==
X-Google-Dkim-Signature	v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc:content-transfer-encoding; bh=G0QviTpIcU41gtdN0Bs/bodEqXLfOYlFPzXtuuhG02A=; b=P34nAp+Ubm3EGGIH+q0kJ8xBGva9ABByym9v1vV9ASUeFPu4LjfN9E6YqJ4qnihuGm dsWYQWVqGUU3oU0uIyD1i04IixNcJ4UsbbuBH7LoKYytRepGZnQLqLcXttfsTsdu9pTu kqBkvPjcwOK+YgmVy38A1GJgDLk2GW2MQxUfY5W77fbqAMZiIDvQKhXHWc2ZxtJ/QLCo X4h6ifSzXWnkFNjfWaeLQ3kMI8znEkNzGHXNAQECXJYByLlGoFsIQdQyj+QOBedKPNGm maEcpp3+UXwGZgri76Wxd6nD1RN+C7y6km99DZCS++pMwFDs2x5vzIt4zFIK9rY6raPs LHIw==
X-Gm-Message-State	AIVw1118nA94Cj8gJ663GxzI6fpgGwQn8UQrW/Mp1Z7005Q65myZNsrO i0PWIbTeiVaPZKKyy2Elmfwu0AGJgw==
X-Received	by 10.36.90.203 with SMTP id v194mr15965974ita.48.1499267238778; Wed, 05 Jul 2017 08:07:18 -0700 (PDT)
MIME-Version	1.0
X-Google-Sender-Auth	wGchPNu1HwZdLmuaYXR9g9jgyo0
Content-Type	text/plain; charset="UTF-8"
Content-Transfer-Encoding	quoted-printable
Sender	robomod@news.nic.it
List-ID	<linux-kernel.vger.kernel.org>
X-Mailing-List	linux-kernel@vger.kernel.org
Approved	robomod@news.nic.it
Lines	52
Organization	linux.* mail to news gateway
X-Original-Cc	Ulf Hansson <ulf.hansson@linaro.org>, Linus Walleij <linus.walleij@linaro.org>, Linux MMC List <linux-mmc@vger.kernel.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
X-Original-Date	Wed, 5 Jul 2017 17:07:17 +0200
X-Original-Message-ID	<CAMuHMdU179SU6ieCBLLRCa=qFEqEF5a5Ut0gdpKyFGCqMOO8aw@mail.gmail.com>
X-Original-References	<1499245637-28870-1-git-send-email-geert@linux-m68k.org> <CAK8P3a1ANjdbHt0x=o2WXB9AHbSj1xr1B-GOqzoKUagma3bBnA@mail.gmail.com>
X-Original-Sender	linux-kernel-owner@vger.kernel.org
Xref	csiph.com linux.kernel:1681573

Show key headers only | View raw

Hi Arnd,

On Wed, Jul 5, 2017 at 2:06 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wed, Jul 5, 2017 at 11:07 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> With gcc 4.1.2:
>>
>>     drivers/mmc/core/block.c: In function ‘mmc_blk_issue_drv_op’:
>>     drivers/mmc/core/block.c:1178: warning: ‘ret’ may be used uninitialized in this function
>>
>> Indeed, if mq_rq->ioc_count is zero, an uninitialized value will be
>> stored in mq_rq->drv_op_result and passed to blk_end_request_all().
>>
>> Can mq_rq->ioc_count be zero?
>>   - mmc_blk_ioctl_cmd() sets ioc_count to 1, so this is safe,
>>   - mmc_blk_ioctl_multi_cmd() obtains ioc_count from user space in
>>     response to the MMC_IOC_MULTI_CMD ioctl, and does allow zero.
>>
>> Preinitialize ret to -EINVAL to fix this for current and future callers.
>>
>> Fixes: 0493f6fe5bdee8ac ("mmc: block: Move boot partition locking into a driver op")
>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> ---
>> I assume -EINVAL is the error we want to return here if ioc_count is
>> zero.
>> Or should it return success (zero), like mmc_blk_ioctl_multi_cmd() used
>> to do?
>
> I would suggest adding the initialization after "case MMC_DRV_OP_IOCTL:"
> instead, to keep getting compile-time checks on the state of the 'ret'
> variable. In that case, returning '0' is probably good.

Makes sense, I actually did consider that, but went with (shared) -EINVAL.

Will send v2 shortly.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[PATCH 1/2] mmc: block: Preinitialize ret in mmc_blk_issue_drv_op() Geert Uytterhoeven <geert@linux-m68k.org> - 2017-07-05 11:10 +0200
  [PATCH 2/2] mmc: block: Let MMC_IOC_MULTI_CMD return zero again for zero entries Geert Uytterhoeven <geert@linux-m68k.org> - 2017-07-05 11:10 +0200
  Re: [PATCH 1/2] mmc: block: Preinitialize ret in mmc_blk_issue_drv_op() Arnd Bergmann <arnd@arndb.de> - 2017-07-05 14:10 +0200
    Re: [PATCH 1/2] mmc: block: Preinitialize ret in mmc_blk_issue_drv_op() Geert Uytterhoeven <geert@linux-m68k.org> - 2017-07-05 17:10 +0200

csiph-web