[Pkg-clamav-devel] Bug#1073128: Bug#1073128: clamav: unaligned access on armhf architecture

[Sebastian Andrzej Siewior]

On 2024-06-13 09:34:14 [+0200], Gianfranco Costamagna wrote:
> Hello, in Ubuntu, where the kernel is configured to forbid unaligned accesses on armhf, the package FTBFS
> (this should be reproducible also on some Debian buildd machines, this is why I'm reporting as serious severity)

Isn't ARMv6+ double of unaligned access? armhf requires VFP so that
should be something later. Or am I wrong here?

> example of failure:
> https://launchpadlibrarian.net/734963041/buildlog_ubuntu-oracular-armhf.clamav_1.3.1+dfsg-3ubuntu1_BUILDING.txt.gz
…
> The following patch makes sure the unaligned access is fixed:
> 
> Description: resolve armhf failure to build from source.

Instead of arguing with me, you could forward it directly to upstream
and give a pointer to apply whatever upsteams did.

> Author: Vladimir Petko <vladimir.petko at canonical.com>

> --- a/libclamav/special.c
> +++ b/libclamav/special.c
> @@ -48,7 +48,8 @@
> 
>  int cli_check_mydoom_log(cli_ctx *ctx)
>  {
> -    const uint32_t *record;
> +    const uint32_t record[16];
> +    const uint32_t mask = 0xffffffff;
please get rid of mask and add your data pointer here.

>      uint32_t check, key;
>      fmap_t *map         = ctx->fmap;
>      unsigned int blocks = map->len / (8 * 4);
> @@ -59,14 +60,20 @@
>      if (blocks > 5)
>          blocks = 5;
> 
> -    record = fmap_need_off_once(map, 0, 8 * 4 * blocks);
> -    if (!record)
> +    // returns unaligned memory block
> +    const char* data = fmap_need_off_once(map, 0, 8 * 4 * blocks);
> +    if (!data)
>          return CL_CLEAN;
> +
>      while (blocks) { /* This wasn't probably intended but that's what the current code does anyway */
> -        if (record[--blocks] == 0xffffffff)
> +        unsigned int offset = --blocks;
> +        offset *=sizeof(uint32_t);
> +        // safe (but slow) on unaligned memory
> +        if (!memcmp(&data[offset], &mask, sizeof(uint32_t)))
>              return CL_CLEAN;

something like
|      while (blocks) { /* This wasn't probably intended but that's what the current code does anyway */
|         uint32_t local;
|
|	  memcpy(&local, &data[--blocks * sizeof(uint32_t)], sizeof(uint32_t));
|         if (local == 0xffffffff)
|              return CL_CLEAN;

I am halfway tempted to suggest a packed-wrapper but this would be best
to consult upstream first.

>      }
> -
> +    // copy into aligned array to perform bit operations
> +    memcpy(record, data, sizeof(record));
>      key   = ~be32_to_host(record[0]);
>      check = (be32_to_host(record[1]) ^ key) +
>              (be32_to_host(record[2]) ^ key) +

Sebastian