All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bartosz Golaszewski <brgl@bgdev.pl>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Jonathan Corbet <corbet@lwn.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J . Wysocki" <rafael@kernel.org>,
	Guenter Roeck <linux@roeck-us.net>,
	Jean Delvare <jdelvare@suse.com>,
	Linux Doc Mailing List <linux-doc@vger.kernel.org>,
	lkml <linux-kernel@vger.kernel.org>,
	linux-hwmon@vger.kernel.org,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>
Subject: Re: [PATCH v2 4/6] devres: handle zero size in devm_kmalloc()
Date: Mon, 12 Apr 2021 21:23:06 +0200	[thread overview]
Message-ID: <CAMRc=MfU8QFPWu_e6oQgtq0cBA34kCSi9=9-1iaFW4gW+yXN6A@mail.gmail.com> (raw)
In-Reply-To: <CAKdAkRRRyC3x39CDRq_3ur1=MWuUug6ov-NWmSBc3h+XyRcsgQ@mail.gmail.com>

On Sun, Apr 11, 2021 at 5:21 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Hi Bartosz,
>
> On Mon, Jun 29, 2020 at 1:56 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > Make devm_kmalloc() behave similarly to non-managed kmalloc(): return
> > ZERO_SIZE_PTR when requested size is 0. Update devm_kfree() to handle
> > this case.
>
> This is wrong if you consider devm_krealloc API that you added. The
> premise of devm_krealloc() is that it does not disturb devres "stack",
> however in this case there is no entry in the stack. Consider:
>
>         ptr = devm_kzalloc(dev, 0, GFP_KERNEL);
>         ...
>         more devm API calls
>         ...
>
>         /* This allocation will be on top of devm stack, not bottom ! */
>         ptr = devm_krealloc(dev, ptr, 16, GFP_KERNEL);
>
> And also:
>
>         ptr = devm_kzalloc(dev, 16, GFP_KERNEL);
>         ...
>         more devm API calls
>         ...
>         /* Here we lose out position */
>         ptr = devm_krealloc(dev, ptr, 0, GFP_KERNEL);
>         ...
>         /* and now our memory allocation will be released first */
>         ptr = devm_krealloc(dev, ptr, 16, GFP_KERNEL);
>
>
> IMO special-casing 0-size allocations for managed memory allocations
> should not be done.
>
> Thanks.
>
> --
> Dmitry

You're right about the ordering being lost. At the same time
allocating 0 bytes is quite a special case and should result in
returning ZERO_SIZE_PTR as the fault dump resulting from its
dereference will indicate what the bug is.

I need to give it a thought because I'm not yet sure what the right
solution would be. Let me get back to you.

Bartosz

  reply	other threads:[~2021-04-12 19:23 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-29  6:50 [PATCH v2 0/6] devres: provide and use devm_krealloc() Bartosz Golaszewski
2020-06-29  6:50 ` [PATCH v2 1/6] devres: remove stray space from devm_kmalloc() definition Bartosz Golaszewski
2020-06-29  6:50 ` [PATCH v2 2/6] devres: move the size check from alloc_dr() into a separate function Bartosz Golaszewski
2020-06-29  6:50 ` [PATCH v2 3/6] device: remove 'extern' attribute from function prototypes in device.h Bartosz Golaszewski
2020-06-29  6:50 ` [PATCH v2 4/6] devres: handle zero size in devm_kmalloc() Bartosz Golaszewski
2020-07-10 13:46   ` Jon Hunter
2020-07-10 16:03     ` Bartosz Golaszewski
2020-07-10 16:11       ` Jon Hunter
2020-07-10 16:24         ` Bartosz Golaszewski
2020-07-10 16:30           ` Jon Hunter
2021-04-11  3:21   ` Dmitry Torokhov
2021-04-12 19:23     ` Bartosz Golaszewski [this message]
2020-06-29  6:50 ` [PATCH v2 5/6] devres: provide devm_krealloc() Bartosz Golaszewski
2020-07-02 12:42   ` Greg Kroah-Hartman
2020-07-02 13:11     ` Bartosz Golaszewski
2020-07-06 16:38       ` Bartosz Golaszewski
2020-07-06 16:41         ` Greg Kroah-Hartman
2020-07-10 13:32         ` Greg Kroah-Hartman
2020-06-29  6:50 ` [PATCH v2 6/6] hwmon: pmbus: use more devres helpers Bartosz Golaszewski
2020-06-29 16:32   ` Guenter Roeck
2020-07-02 12:44   ` Greg Kroah-Hartman
2020-07-02 13:06     ` Bartosz Golaszewski
2020-07-02 14:29     ` Guenter Roeck
2020-07-02 12:44 ` [PATCH v2 0/6] devres: provide and use devm_krealloc() Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAMRc=MfU8QFPWu_e6oQgtq0cBA34kCSi9=9-1iaFW4gW+yXN6A@mail.gmail.com' \
    --to=brgl@bgdev.pl \
    --cc=bgolaszewski@baylibre.com \
    --cc=corbet@lwn.net \
    --cc=dmitry.torokhov@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jdelvare@suse.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=rafael@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.