From: Richard Yao <ryao@gentoo•org>
To: David Miller <davem@davemloft•net>
Cc: netdev@vger•kernel.org
Subject: Re: Latest version of zero-copy fix
Date: Mon, 10 Feb 2014 20:31:11 -0500 [thread overview]
Message-ID: <52F97D5F.4040100@gentoo.org> (raw)
In-Reply-To: <20140210.152150.574073124161003333.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 2137 bytes --]
On 02/10/2014 06:21 PM, David Miller wrote:
>
> So now the patch only tests is_vmalloc_addr(), did you test this
> version with the situation that triggers the given backtrace?
>
> [<ffffffff814878ce>] p9_virtio_zc_request+0x45e/0x510
> [<ffffffff814814ed>] p9_client_zc_rpc.constprop.16+0xfd/0x4f0
> [<ffffffff814839dd>] p9_client_read+0x15d/0x240
> [<ffffffff811c8440>] v9fs_fid_readn+0x50/0xa0
> [<ffffffff811c84a0>] v9fs_file_readn+0x10/0x20
> [<ffffffff811c84e7>] v9fs_file_read+0x37/0x70
> [<ffffffff8114e3fb>] vfs_read+0x9b/0x160
> [<ffffffff81153571>] kernel_read+0x41/0x60
> [<ffffffff810c83ab>] copy_module_from_fd.isra.34+0xfb/0x180
>
> This is reading from v9fs into a module address.
>
> It's going generate the same backtrace with your fix.
>
> I don't think the situation was sufficiently explained to Linus. In
> fact, I didn't see the above backtrace mentioned at all.
>
I have tested it against Linux 3.13.0 and I can confirm that it solves
the problem. In fact, this was the original patch in December before I
made the last minute change to is_module_or_vmalloc_addr() only after
deciding that is_vmalloc_addr() was not general enough.
Linus' response had two key points. Linus' first point was that my
generalization of is_vmalloc_addr() to is_module_or_vmalloc_addr() was
unnecessary to fix this problem because the order of events is as follows:
1. We allocate a temporary buffer with vmalloc().
2. We read the module into that buffer.
3. We copy the module from that buffer into its final location.
Linus' second point was that my generalization permitted IO to be done
to a region of memory where IO operations should never be done. When I
wrote this back in December, I wanted to write as general a fix as
possible because Alexander Graf had pointed out to me that there had
Will Deacon had written a patch that should have included this, but
neglected it and I was afraid that that would become a cycle.
Linus' clear explanation of why this is a bad idea changed my mind, so I
submitted the original form of this patch with an updated commit message.
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
next prev parent reply other threads:[~2014-02-11 1:31 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-10 23:21 Latest version of zero-copy fix David Miller
2014-02-11 1:31 ` Richard Yao [this message]
2014-02-11 1:48 ` David Miller
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=52F97D5F.4040100@gentoo.org \
--to=ryao@gentoo$(echo .)org \
--cc=davem@davemloft$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox