From: Mike Snitzer <snitzer@redhat.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: Kent Overstreet <kent.overstreet@gmail.com>,
torvalds@linux-foundation.org, linux-kernel@vger.kernel.org
Subject: Re: dm: Use kzalloc for all structs with embedded biosets/mempools
Date: Tue, 5 Jun 2018 10:45:07 -0400 [thread overview]
Message-ID: <20180605144506.GA13100@redhat.com> (raw)
In-Reply-To: <e78b0f9f-d4f9-d865-0727-5e1ed074928d@kernel.dk>
On Tue, Jun 05 2018 at 10:22P -0400,
Jens Axboe <axboe@kernel.dk> wrote:
> On 6/5/18 3:26 AM, Kent Overstreet wrote:
> > mempool_init()/bioset_init() require that the mempools/biosets be zeroed
> > first; they probably should not _require_ this, but not allocating those
> > structs with kzalloc is a fairly nonsensical thing to do (calling
> > mempool_exit()/bioset_exit() on an uninitialized mempool/bioset is legal
> > and safe, but only works if said memory was zeroed.)
> >
> > Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com>
> > ---
> >
> > Linus,
> >
> > I fucked up majorly on the bioset/mempool conversion - I forgot to check that
> > everything biosets/mempools were being embedded in was actually being zeroed on
> > allocation. Device mapper currently explodes, you'll probably want to apply this
> > patch post haste.
> >
> > I have now done that auditing, for every single conversion - this patch fixes
> > everything I found. There do not seem to be any incorrect ones outside of device
> > mapper...
> >
> > We'll probably want a second patch that either a) changes
> > bioset_init()/mempool_init() to zero the passed in bioset/mempool first, or b)
> > my preference, WARN() or BUG() if they're passed memory that isn't zeroed.
>
> Odd, haven't seen a crash, but probably requires kasan or poisoning to
> trigger anything? Mike's tree also had the changes, since they were based
> on the block tree.
>
> I can queue this up and ship it later today. Mike, you want to review
> this one?
Yes, looks good.
>From the start of revisiting these changes last week, Kent and I
discussed whether it was safe to call mempool_exit() even if
mempool_init() failed or was never called. He advised that it was so
long as the containing structure was zeroed. But I forgot to audit that
aspect. So this was an oversight by both of us.
DM core uses kvzalloc_node for struct mapped_device and cache, crypt,
integrity, verity-fec and zoned targets are already using kzalloc as
needed.
Acked-by: Mike Snitzer <snitzer@redhat.com>
next prev parent reply other threads:[~2018-06-05 14:45 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-05 9:26 [PATCH] dm: Use kzalloc for all structs with embedded biosets/mempools Kent Overstreet
2018-06-05 14:22 ` Jens Axboe
2018-06-05 14:35 ` David Sterba
2018-06-05 14:42 ` Jens Axboe
2018-06-05 15:08 ` David Sterba
2018-06-05 14:45 ` Mike Snitzer [this message]
2018-06-05 14:48 ` Jens Axboe
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=20180605144506.GA13100@redhat.com \
--to=snitzer@redhat.com \
--cc=axboe@kernel.dk \
--cc=kent.overstreet@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=torvalds@linux-foundation.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.