Re: [PULL 00/34] s390x update

[David Hildenbrand <david@redhat.com>]

On 20.09.19 13:51, David Hildenbrand wrote:
> On 20.09.19 13:00, Cornelia Huck wrote:
>> On Fri, 20 Sep 2019 11:45:18 +0100
>> Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>>> On Thu, 19 Sep 2019 at 13:41, Cornelia Huck <cohuck@redhat.com> wrote:
>>>>
>>>> The following changes since commit f8c3db33a5e863291182f8862ddf81618a7c6194:
>>>>
>>>>   target/sparc: Switch to do_transaction_failed() hook (2019-09-17 12:01:00 +0100)
>>>>
>>>> are available in the Git repository at:
>>>>
>>>>   https://github.com/cohuck/qemu tags/s390x-20190919
>>>>
>>>> for you to fetch changes up to 37105adebeb28e60da3cb1ef82231d7ed8d23589:
>>>>
>>>>   Merge tag 'tags/s390-ccw-bios-2019-09-18' into s390-next-staging (2019-09-19 12:04:01 +0200)
>>>>
>>>> ----------------------------------------------------------------
>>>> - bugfixes in tcg and the ccw bios
>>>> - gen15a is called z15
>>>> - officially require a 3.15 kernel or later for kvm
>>>>
>>>> ----------------------------------------------------------------  
>>>
>>> Hi -- I'm afraid this pullreq results in new warnings from
>>> the runtime-sanitizer build when 'make check' is run:
>>> MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
>>> QTEST_QEMU_BINARY=s390x-softmmu/qemu-system-s390x
>>> QTEST_QEMU_IMG=qemu-img tests
>>> /boot-serial-test -m=quick -k --tap < /dev/null |
>>> ./scripts/tap-driver.pl --test-name="boot-serial-test"
>>> /home/petmay01/linaro/qemu-for-merges/target/s390x/mem_helper.c:293:17:
>>> runtime error: null pointer passed as argument 1, which is declared to
>>> never be null
>>> /usr/include/string.h:47:14: note: nonnull attribute specified here
>>> /home/petmay01/linaro/qemu-for-merges/target/s390x/mem_helper.c:293:32:
>>> runtime error: null pointer passed as argument 2, which is declared to
>>> never be null
>>>
>>> (and the same warnings for a few other tests).
>>>
>>> Looks like you sometimes can pass NULL pointers to the source
>>> and destination of memmove(). This isn't permitted by the
>>> standard even in the case where the size argument is zero.
>>>
>>> thanks
>>> -- PMM
>>
>> David, can you take a look?
>>
> 
> I would have assumed these pointers are ignored in case the length is
> zero, too (the only way this can happen). Easy to fix.
> 


diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index e50cec9263..ef8e0c20a7 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -290,17 +290,23 @@ static void access_memmove(CPUS390XState *env, S390Access *desta,
 
     if (srca->size1 == desta->size1) {
         memmove(desta->haddr1, srca->haddr1, srca->size1);
-        memmove(desta->haddr2, srca->haddr2, srca->size2);
+        if (likely(srca->size2)) {
+            memmove(desta->haddr2, srca->haddr2, srca->size2);
+        }
     } else if (srca->size1 < desta->size1) {
         diff = desta->size1 - srca->size1;
         memmove(desta->haddr1, srca->haddr1, srca->size1);
         memmove(desta->haddr1 + srca->size1, srca->haddr2, diff);
-        memmove(desta->haddr2, srca->haddr2 + diff, desta->size2);
+        if (likely(desta->size2)) {
+            memmove(desta->haddr2, srca->haddr2 + diff, desta->size2);
+        }
     } else {
         diff = srca->size1 - desta->size1;
         memmove(desta->haddr1, srca->haddr1, desta->size1);
         memmove(desta->haddr2, srca->haddr1 + desta->size1, diff);
-        memmove(desta->haddr2 + diff, srca->haddr2, srca->size2);
+        if (likely(srca->size2)) {
+            memmove(desta->haddr2 + diff, srca->haddr2, srca->size2);
+        }
     }
 }

For "s390x/tcg: Fault-safe memmove" should do the trick. Will test.

-- 

Thanks,

David / dhildenb