| |
|
 |
|
 |
|
|
|
| |
|
|
xen-devel
Re: [Xen-devel] Two small patches related to xenfb
Rafal Wojtczuk writes ("[Xen-devel] Two small patches related to xenfb"):
> Two minor issues:
> row_stride_div0.patch: a malicious frontend can send row_stride==0 and force
> qemu-dm to perform division by 0
> vnc_resize_doublecheck.patch: there is an unchecked multiplication when
> calculating framebuffer size. Cs 17630 sanitizes framebuffer dimensions
> passed by the frontend, so most probably no integer overflow can happen, but
> there should be a check for overflow close to the actual computation (to
> make code review easier and to cope with other codepaths in the future).
Thanks. Your patch wasn't quite right in a couple of ways but I have
fixed it up and applied it to qemu-xen-unstable. I'll cherry pick it
into 3.3 after I get a successful test run.
> Diffs against xen-3.2-testing.hg.
Thanks but xen-3.2 is pretty much out of our support life cycle now.
We would suggest you use 3.3 instead.
> +static int mult_overflows(int x, int y)
> +{
> + if (x<=0 || y<=0 || x*y<=0 || x>((unsigned int)(-1))/y)
This isn't correct.
* The use of (unsigned int)(-1) is strange; you should use INT_MAX
(and not ~(unsigned int)0 either, as we wanted the figure for
_signed_ overflow).
* Multiplying x*y is undefined behaviour if it's an overflow;
that can in theory lead the compiler to `prove' that you know
that there is no overflow and conclude that the other tests cannot
fail. You should not do this test at all. The division is
sufficient.
> + if (mult_overflows(w, h) || mult_overflows(w*h, vs->depth) ||
> + mult_overflows(h, sizeof(vs->dirty_row[0])) {
Evidently you didn't compile this because it has a missing
parenthesis.
Also, you should include a Signed-Off-By line like this
Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
except with your name and email address. This indicates that you are
certifying the code as suitable (copyright-wise and so on) for
inclusion in Xen. You can find the precise meaning in the Linux
upstream kernel tree (Documentation/SubmittingPatches).
In this case, and since your patch was so small, I took your message
to grant the relevant permissions.
Ian.
>From Documentation/SubmittingPatches:
Developer's Certificate of Origin 1.1
By making a contribution to this project, I certify that:
(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or
(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or
(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.
(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.
--
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
|
|
| |
|
Home