From: Michael Kelley <mikelley@microsoft.com>
To: "lantianyu1986@gmail.com" <lantianyu1986@gmail.com>,
KY Srinivasan <kys@microsoft.com>,
Haiyang Zhang <haiyangz@microsoft.com>,
Stephen Hemminger <sthemmin@microsoft.com>,
"sashal@kernel.org" <sashal@kernel.org>,
"tglx@linutronix.de" <tglx@linutronix.de>,
"mingo@redhat.com" <mingo@redhat.com>,
"bp@alien8.de" <bp@alien8.de>, "hpa@zytor.com" <hpa@zytor.com>,
"x86@kernel.org" <x86@kernel.org>,
"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
"alex.williamson@redhat.com" <alex.williamson@redhat.com>,
"cohuck@redhat.com" <cohuck@redhat.com>
Cc: Tianyu Lan <Tianyu.Lan@microsoft.com>,
"linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"kvm@vger.kernel.org" <kvm@vger.kernel.org>
Subject: RE: [PATCH] x86/Hyper-V: Fix overflow issue in the fill_gva_list()
Date: Fri, 30 Aug 2019 17:41:06 +0000 [thread overview]
Message-ID: <DM5PR21MB0137B7C2AAD0FC65CB3E1306D7BD0@DM5PR21MB0137.namprd21.prod.outlook.com> (raw)
In-Reply-To: <20190830061540.211072-1-Tianyu.Lan@microsoft.com>
From: lantianyu1986@gmail.com Sent: Thursday, August 29, 2019 11:16 PM
>
> From: Tianyu Lan <Tianyu.Lan@microsoft.com>
>
> fill_gva_list() populates gva list and adds offset
> HV_TLB_FLUSH_UNIT(0x1000000) to variable "cur"
> in the each loop. When diff between "end" and "cur" is
> less than HV_TLB_FLUSH_UNIT, the gva entry should
> be the last one and the loop should be end.
>
> If cur is equal or greater than 0xFF000000 on 32-bit
> mode, "cur" will be overflow after adding HV_TLB_FLUSH_UNIT.
> Its value will be wrapped and less than "end". fill_gva_list()
> falls into an infinite loop and fill gva list out of
> border finally.
>
> Set "cur" to be "end" to make loop end when diff is
> less than HV_TLB_FLUSH_UNIT and add HV_TLB_FLUSH_UNIT to
> "cur" when diff is equal or greater than HV_TLB_FLUSH_UNIT.
> Fix the overflow issue.
Let me suggest simplifying the commit message a bit. It
doesn't need to describe every line of the code change. I think
it should also make clear that the same problem could occur on
64-bit systems with the right "start" address. My suggestion:
When the 'start' parameter is >= 0xFF000000 on 32-bit
systems, or >= 0xFFFFFFFF'FF000000 on 64-bit systems,
fill_gva_list gets into an infinite loop. With such inputs,
'cur' overflows after adding HV_TLB_FLUSH_UNIT and always
compares as less than end. Memory is filled with guest virtual
addresses until the system crashes
Fix this by never incrementing 'cur' to be larger than 'end'.
>
> Reported-by: Jong Hyun Park <park.jonghyun@yonsei.ac.kr>
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> Fixes: 2ffd9e33ce4a ("x86/hyper-v: Use hypercall for remote
> TLB flush")
The "Fixes:" line needs to not wrap. It's exempt from the
"wrap at 75 columns" rule in order to simplify parsing scripts.
The code itself looks good.
Michael
next prev parent reply other threads:[~2019-08-30 17:41 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-30 6:15 [PATCH] x86/Hyper-V: Fix overflow issue in the fill_gva_list() lantianyu1986
2019-08-30 17:41 ` Michael Kelley [this message]
2019-09-02 12:46 ` Tianyu Lan
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=DM5PR21MB0137B7C2AAD0FC65CB3E1306D7BD0@DM5PR21MB0137.namprd21.prod.outlook.com \
--to=mikelley@microsoft.com \
--cc=Tianyu.Lan@microsoft.com \
--cc=alex.williamson@redhat.com \
--cc=bp@alien8.de \
--cc=cohuck@redhat.com \
--cc=gregkh@linuxfoundation.org \
--cc=haiyangz@microsoft.com \
--cc=hpa@zytor.com \
--cc=kvm@vger.kernel.org \
--cc=kys@microsoft.com \
--cc=lantianyu1986@gmail.com \
--cc=linux-hyperv@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=sashal@kernel.org \
--cc=sthemmin@microsoft.com \
--cc=tglx@linutronix.de \
--cc=x86@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).