On Mon, 2021-06-14 at 13:06 +0200, Michal Privoznik wrote:
> In a few occasions in tests we pass INT_MAX to
> virFileReadLimFD(). This is not safe because virFileReadAll()
> will call virFileReadLimFD() under the hood which takes the limit
> and adds 1 to it.
Calling virFileReadAll with "INT_MAX - 1" looks funny. Is it possible
to check for "maxlen >= INT_MAX" in virFileReadLimFD instead?
> And since we use signed integer for all of this
> an overflow will occur.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> tests/networkxml2firewalltest.c | 2 +-
> tests/testutils.c | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tests/networkxml2firewalltest.c
> b/tests/networkxml2firewalltest.c
> index 91336a0c55..facbc20a0c 100644
> --- a/tests/networkxml2firewalltest.c
> +++ b/tests/networkxml2firewalltest.c
> @@ -176,7 +176,7 @@ mymain(void)
>
> basefile =
> g_strdup_printf("%s/networkxml2firewalldata/base.args", abs_srcdir);
>
> - if (virFileReadAll(basefile, INT_MAX, &baseargs) < 0)
> + if (virFileReadAll(basefile, INT_MAX - 1, &baseargs) < 0)
> return EXIT_FAILURE;
>
> DO_TEST("nat-default");
> diff --git a/tests/testutils.c b/tests/testutils.c
> index eb3bd48b6a..4a63c6cc37 100644
> --- a/tests/testutils.c
> +++ b/tests/testutils.c
> @@ -313,7 +313,7 @@ virTestLoadFileJSON(const char *p, ...)
> if (!(path = virTestLoadFileGetPath(p, ap)))
> goto cleanup;
>
> - if (virFileReadAll(path, INT_MAX, &jsonstr) < 0)
> + if (virFileReadAll(path, INT_MAX - 1, &jsonstr) < 0)
> goto cleanup;
>
> if (!(ret = virJSONValueFromString(jsonstr)))
> @@ -562,7 +562,7 @@ virTestCompareToFileFull(const char *actual,
> if (virTestLoadFile(filename, &filecontent) < 0 &&
> !virTestGetRegenerate())
> return -1;
> } else {
> - if (virFileReadAll(filename, INT_MAX, &filecontent) < 0 &&
> !virTestGetRegenerate())
> + if (virFileReadAll(filename, INT_MAX - 1, &filecontent) < 0
> && !virTestGetRegenerate())
> return -1;
> }
>
