Thanks for your polish. I will update the patch as soon as possible. I have tested the patch in my environment. What I use before is : virsh qemu-monitor-command xxxx --hmp "info tlb" With this patch, when I query tlb, after some time, the console only print: "error: No complete monitor response found in 10485760 bytes: Numerical result out of range". It is because libvirt have noticed this scene and try to avoid memory denial-of-service.
Besides, I login in the corresponding guestos which is win10 with 29G memory and find that a test tool called "BurnLnTest" is running busily. Maybe this test tool enlarge the size of tlb. > -----Original Message----- > From: Markus Armbruster [mailto:arm...@redhat.com] > Sent: Tuesday, July 24, 2018 8:08 PM > To: liujunjie (A) <liujunji...@huawei.com> > Cc: wangxin (U) <wangxinxin.w...@huawei.com>; Gonglei (Arei) > <arei.gong...@huawei.com>; Huangweidong (C) > <weidong.hu...@huawei.com>; qemu-devel@nongnu.org > Subject: Re: [Qemu-devel] [PATCH] qstring: Fix integer overflow > > "liujunjie (A)" <liujunji...@huawei.com> writes: > > > Even using gdb command: set print elements 0, is still too large to print > > the > whole string. > > So I try to obtain the string in another way. > > After several attempts(not easy in fact), I finally obtain the real length. > > The > way is as follows: > > (gdb) p (int *)str > > $1 = (int *) 0x7f13a2e67010 > > (gdb) p *(char*) (0x7f13a2e67010 + 0x8B0FD63FF)@192 > > $2 = "--W\r\nffffffffffd17000: 00000000fec00000 > XG-DACT-W\r\nffffffffffd18000: ", '0' <repeats 11 times>, "77000 > XG-DA---W\r\nffffffffffd19000: ", '0 "78000 XG-DA---W\r\nffffffffffd1a000: ", > '0' > <repeats 11 times>, "79000 XG-DA---W\r\n\000\000" > > With \000 at the end, we can find out the length is 0x8B0FD63FF + 192 - 2, > that is 37329134781. > > Awesome, thanks! > > 37329134781 is 0x8B0FD64BD. That's almost 35 GiB. How on earth can > "info tlb" print several Gigabytes? That's a not a sane use of QMP! No > excuse for crashing, of course. > > > With this length, we can write a simple c code demo to reproduce the result > we obtain before. Such as: > > ----------------------------- > > #include<stdio.h> > > int main() > > { > > char * tmp = ""; > > size_t a = 37329134781; > > int end = a; > > size_t b = end; > > printf("%zu", b) > > return 0; > > } > > ----------------------------- > > Yes. > > When strlen(str) exceeds INT_MAX - 1 in qstring_from_str(), the @end > argument for qstring_from_substr() gets truncated. This is what appeared to > happen in your case: it got truncated to -1325570883. > > qstring_from_substr() has an (unstated) precondition: start >= 0 && end > >= 0 && end - start >= -1. This precondition is violated. end - start > + 1 gets sign-extended to a huge value, and g_malloc() duly fails. > > All callers of qstring_from_substr() outside tests pass arguments of type > size_t > or ptrdiff_t. They'd fail just like qstring_from_str() for sub-strings > exceeding 2 > GiB. Your patch fixes them all. Good, I'll take your patch, but the commit > message needs a bit of polish. Here's my attempt: > > qstring: Fix qstring_from_substr() not to provoke int overflow > > qstring_from_substr() parameters @start and @end are of type int. > blkdebug_parse_filename(), blkverify_parse_filename(), nbd_parse_uri(), > and qstring_from_str() pass @end values of type size_t or ptrdiff_t. > Values exceeding INT_MAX get truncated, with possibly disastrous > results. > > Such huge substrings seem unlikely, but we found one in a core dump, > where "info tlb" executed via QMP's human-monitor-command apparently > produced 35 GiB of output. > > Fix by changing the parameters size_t. > > If this works for you, please reply with your corrected Signed-off-by. > > However, please note that allocating the correct 35 GiB may not behave much > better for you than allocating 16 EiB. I doubt it'll succeed, and if it > does, then > memcpy() will dirty 35 GiB of virtual memory, which is unlikely to end well. > > Are you sure "info tlb" is supposed to print that much? Or is there another > bug still in hiding that somehow enlarged this string?
