Received-SPF: pass (google.com: domain of linux-kernel+bounces-110131-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <7307842e-769e-92df-800a-0bf78fe0b70b@linux.ibm.com>
Date: Thu, 21 Mar 2024 10:14:40 -0400
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Subject: Re: [PATCH v3 0/5] s390/vfio-ap: ap_config sysfs attribute for
 mdevctl automation
To: Heiko Carstens <hca@linux.ibm.com>,
        Anthony Krowiak <akrowiak@linux.ibm.com>
Cc: linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org,
        pasic@linux.ibm.com, borntraeger@de.ibm.com, agordeev@linux.ibm.com,
        gor@linux.ibm.com
References: <20240313205807.30554-1-jjherne@linux.ibm.com>
 <48d70b2c-788c-4ff0-9a03-c063aabdec07@linux.ibm.com>
 <20240319111233.23303-D-hca@linux.ibm.com>
Content-Language: en-US
From: "Jason J. Herne" <jjherne@linux.ibm.com>
In-Reply-To: <20240319111233.23303-D-hca@linux.ibm.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit


On 3/19/24 7:12 AM, Heiko Carstens wrote:

> With gcc gcc 13.2.0 / binutils 2.40.90.20230730 I get this (defconfig):
> 
>    CC [M]  drivers/s390/crypto/vfio_ap_ops.o
> In file included from ./include/linux/cpumask.h:13,
>                   from ./include/linux/smp.h:13,
>                   from ./include/linux/lockdep.h:14,
>                   from ./include/linux/spinlock.h:63,
>                   from ./include/linux/mmzone.h:8,
>                   from ./include/linux/gfp.h:7,
>                   from ./include/linux/mm.h:7,
>                   from ./include/linux/scatterlist.h:8,
>                   from ./include/linux/iommu.h:10,
>                   from ./include/linux/vfio.h:12,
>                   from drivers/s390/crypto/vfio_ap_ops.c:12:
> In function ‘bitmap_copy’,
>      inlined from ‘ap_matrix_copy’ at drivers/s390/crypto/vfio_ap_ops.c:1672:2,
>      inlined from ‘ap_config_store’ at drivers/s390/crypto/vfio_ap_ops.c:1696:2:
> ./include/linux/bitmap.h:253:17: warning: ‘memcpy’ reading 32 bytes from a region of size 0 [-Wstringop-overread]
>    253 |                 memcpy(dst, src, len);
>        |                 ^~~~~~~~~~~~~~~~~~~~~
> In function ‘ap_config_store’:
> cc1: note: source object is likely at address zero
> In function ‘bitmap_copy’,
>      inlined from ‘ap_matrix_copy’ at drivers/s390/crypto/vfio_ap_ops.c:1673:2,
>      inlined from ‘ap_config_store’ at drivers/s390/crypto/vfio_ap_ops.c:1696:2:
> ./include/linux/bitmap.h:253:17: warning: ‘memcpy’ reading 32 bytes from a region of size 0 [-Wstringop-overread]
>    253 |                 memcpy(dst, src, len);
>        |                 ^~~~~~~~~~~~~~~~~~~~~
> In function ‘ap_config_store’:
> cc1: note: source object is likely at address zero
> In function ‘bitmap_copy’,
>      inlined from ‘ap_matrix_copy’ at drivers/s390/crypto/vfio_ap_ops.c:1674:2,
>      inlined from ‘ap_config_store’ at drivers/s390/crypto/vfio_ap_ops.c:1696:2:
> ./include/linux/bitmap.h:253:17: warning: ‘memcpy’ reading 32 bytes from a region of size 0 [-Wstringop-overread]
>    253 |                 memcpy(dst, src, len);
>        |                 ^~~~~~~~~~~~~~~~~~~~~
> In function ‘ap_config_store’:
> cc1: note: source object is likely at address zero

I believe that this is a bogus compiler warning. I cannot reproduce it, 
fwiw.

gcc:     gcc (GCC) 13.2.1 20231205 (Red Hat 13.2.1-6)
binutls  binutils-2.40-14.fc39

make W=1 modules

Here is the supposedly offending code.

drivers/s390/crypto/vfio_ap_ops.c:
1670 static void ap_matrix_copy(struct ap_matrix *dst, struct ap_matrix 
*src)
1671 {
1672         bitmap_copy(dst->apm, src->apm, AP_DEVICES);
1673         bitmap_copy(dst->aqm, src->aqm, AP_DOMAINS);
1674         bitmap_copy(dst->adm, src->adm, AP_DOMAINS);
1675 }

called from drivers/s390/crypto/vfio_ap_ops.c:
1695    /* Save old state */
1696    ap_matrix_copy(&m_old, &matrix_mdev->matrix);


Definition of struct in drivers/s390/crypto/vfio_ap_private.h:
113 struct ap_matrix_mdev {
114         struct vfio_device vdev;
115         struct list_head node;
116         struct ap_matrix matrix;
117         struct ap_matrix shadow_apcb;
118         struct kvm *kvm;
119         crypto_hook pqap_hook;
120         struct mdev_device *mdev;
121         struct ap_queue_table qtable;
122         struct eventfd_ctx *req_trigger;
123         DECLARE_BITMAP(apm_add, AP_DEVICES);
124         DECLARE_BITMAP(aqm_add, AP_DOMAINS);
125         DECLARE_BITMAP(adm_add, AP_DOMAINS);
126 };

drivers/s390/crypto/vfio_ap_private.h:
76 struct ap_matrix {
77         unsigned long apm_max;
78         DECLARE_BITMAP(apm, AP_DEVICES);
79         unsigned long aqm_max;
80         DECLARE_BITMAP(aqm, AP_DOMAINS);
81         unsigned long adm_max;
82         DECLARE_BITMAP(adm, AP_DOMAINS);
83 };

drivers/s390/crypto/ap_bus.h:
22 #define AP_DEVICES 256          /* Number of AP devices. */
23 #define AP_DOMAINS 256          /* Number of AP domains. */

The source object seems to have a well defined size.

A quick web search seems to indicate gcc throws quite a few
Wstringop-overread warnings for valid code. I suspect this is
another example of that.