Skip to content

CLAM-1775: increase max allocation size#685

Closed
m-sola wants to merge 1 commit intoCisco-Talos:mainfrom
m-sola:clam-1775
Closed

CLAM-1775: increase max allocation size#685
m-sola wants to merge 1 commit intoCisco-Talos:mainfrom
m-sola:clam-1775

Conversation

@m-sola
Copy link
Contributor

@m-sola m-sola commented Aug 25, 2022

Increases internal max allocation size to allow support for
files up to 3GB.

Provides a more helpful, less dramatic warning message if this
limit is exceeded.

@lgtm-com
Copy link

lgtm-com bot commented Aug 25, 2022

This pull request introduces 1 alert when merging f90c4be into 4a0382c - view on LGTM.com

new alerts:

  • 1 for Comparison result is always the same

@lgtm-com
Copy link

lgtm-com bot commented Sep 28, 2022

This pull request introduces 1 alert when merging 566c512 into 197113c - view on LGTM.com

new alerts:

  • 1 for Comparison result is always the same

cli_errmsg("cli_malloc(): Attempt to allocate %lu bytes. Please report to https://github.com/Cisco-Talos/clamav/issues\n", (unsigned long int)size);
cli_warnmsg("cli_malloc(): File or section is too large to scan (%lu bytes). \
For your safety, ClamAV limits how much memory an operation can allocate to %lu bytes\n",
(unsigned long int)size, (unsigned long int)CLI_MAX_ALLOCATION);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're we're touching this code, it would be nice to change the %lu's over to to %zu.
You could then remove the cast for the size argument. For CLI_MAX_ALLOCATION, you may use size_t` for the cast, although I'm not sure if it is necessary (if you remove it, just check for compile warnings to be sure).

Same goes for cli_calloc, cli_realloc, and cli_realloc2, where the size parameter is a size_t.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this and the other print statements where you changed from %llu to %zu, you will also need to remove the (unsigned long int) cast, or else we'll have compiler warnings on systems where size_t is not equal to unsigned long int.

(size_t)(sb) <= (size_t)(bb_size))

#define CLI_MAX_ALLOCATION (182 * 1024 * 1024)
#define CLI_MAX_ALLOCATION ((uint32_t) 1024 * 1024 * 1024 * 3)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the (uint32_t) cast is necessary.

@lgtm-com
Copy link

lgtm-com bot commented Oct 3, 2022

This pull request fixes 2 alerts when merging 30085be into ece5375 - view on LGTM.com

fixed alerts:

  • 2 for Comparison result is always the same

cli_errmsg("cli_malloc(): Attempt to allocate %lu bytes. Please report to https://github.com/Cisco-Talos/clamav/issues\n", (unsigned long int)size);
cli_warnmsg("cli_malloc(): File or section is too large to scan (%lu bytes). \
For your safety, ClamAV limits how much memory an operation can allocate to %lu bytes\n",
(unsigned long int)size, (unsigned long int)CLI_MAX_ALLOCATION);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this and the other print statements where you changed from %llu to %zu, you will also need to remove the (unsigned long int) cast, or else we'll have compiler warnings on systems where size_t is not equal to unsigned long int.

Increases internal max allocation size to allow support for
files up to 3GB.

Provides a more helpful, less dramatic warning message if this
limit is exceeded.
@lgtm-com
Copy link

lgtm-com bot commented Oct 5, 2022

This pull request fixes 2 alerts when merging b65a8e2 into ece5375 - view on LGTM.com

fixed alerts:

  • 2 for Comparison result is always the same

@val-ms
Copy link
Contributor

val-ms commented Oct 20, 2022

After some testing, we've decided to go with #723 for now. We can revisit increasing it further in the future.

@val-ms val-ms closed this Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants