[PATCH 12/12] test/ipsec: fix build with GCC 12
Medvedkin, Vladimir
vladimir.medvedkin at intel.com
Fri Jun 3 17:57:21 CEST 2022
Hi David,
On 03/06/2022 10:41, David Marchand wrote:
> On Fri, Jun 3, 2022 at 9:56 AM Bruce Richardson
> <bruce.richardson at intel.com> wrote:
>>
>> On Fri, Jun 03, 2022 at 09:45:45AM +0200, David Marchand wrote:
>>> Hello Vladimir,
>>>
>>> On Thu, Jun 2, 2022 at 8:42 PM Medvedkin, Vladimir
>>> <vladimir.medvedkin at intel.com> wrote:
>>>>> if (!dst) {
>>>>> rte_pktmbuf_free(m);
>>>>> return NULL;
>>>>> }
>>>>> - if (string != NULL)
>>>>> - rte_memcpy(dst, string, t_len);
>>>>> - else
>>>>> - memset(dst, 0, t_len);
>>>>> + if (string != NULL) {
>>>>> + size_t off;
>>>>> +
>>>>> + for (off = 0; off + string_len < len; off += string_len)
>>>>
>>>> I think it should be off + string_len <= len here, because otherwise, if
>>>> len is a multiple of string_len, the last ret_memcpy (after this loop)
>>>> will copy 0 bytes.
>>>
>>> Changing to off + string_len <= len would trigger an oob access to dst
>>> (by one extra byte)?
>>> Otoh, I don't think it is an issue to have a 0-length call to rte_memcpy.
>>>
The problem here is that if, for example, string_len is 8 bytes and len
is 16, then it will write only 8 bytes.
>> Given this is test code, do we need rte_memcpy for performance over regular
>> libc memcpy? Does fixing the warning become any easier or clearer if libc
>> memcpy is used?
>
> There was a similar proposal in vhost/crypto code.
> I am not a fan to switching to libc memcpy.
> We would be waiving a potential issue in rte_memcpy itself (which
> could also be a problem in how gcc understands this inlined code) or
> in the rte_memcpy caller code.
>
> Here, gcc is probably too picky.
> No path currently leads to oob access on the src string.
>
> Adding a simple hint (see simplified hunk below) seems to help gcc enough:
>
> @@ -554,12 +554,14 @@ struct rte_ipv4_hdr ipv4_outer = {
> };
>
> static struct rte_mbuf *
> -setup_test_string(struct rte_mempool *mpool,
> - const char *string, size_t len, uint8_t blocksize)
> +setup_test_string(struct rte_mempool *mpool, const char *string,
> + size_t string_len, size_t len, uint8_t blocksize)
> {
> struct rte_mbuf *m = rte_pktmbuf_alloc(mpool);
> size_t t_len = len - (blocksize ? (len % blocksize) : 0);
>
> + RTE_VERIFY(len <= string_len);
> +
RTE_VERIFY looks better here to make picky GCC happy.
>
> if (m) {
> memset(m->buf_addr, 0, m->buf_len);
>
>
--
Regards,
Vladimir
More information about the stable
mailing list