On 7/23/24 13:08, Jiri Denemark wrote: > On Tue, Jul 23, 2024 at 12:41:31 +0200, Michal Privoznik wrote: >> On various occasions, virt-host-validate parses /proc/cpuinfo to >> learn about CPU flags (see virHostValidateGetCPUFlags()). It does >> so, by reading the file line by line until the line with CPU >> flags is reached. Then the line is split into individual flags >> (using space as a delimiter) and the list of flags is then >> iterated over. >> >> This works, except for cases when the line with CPU flags is too >> long. Problem is - the line is capped at 1024 bytes and on newer >> CPUs (and newer kernels), the line can be significantly longer. >> I've seen a line that's ~1200 characters long (with 164 flags >> reported). >> >> Switch to unbounded read from the file (getline()). >> >> Resolves: https://issues.redhat.com/browse/RHEL-39969 >> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> >> --- >> tools/virt-host-validate-common.c | 16 ++++------------ >> 1 file changed, 4 insertions(+), 12 deletions(-) >> >> diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c >> index 591143c24d..e5fa6606d2 100644 >> --- a/tools/virt-host-validate-common.c >> +++ b/tools/virt-host-validate-common.c >> @@ -106,21 +106,19 @@ virBitmap *virHostValidateGetCPUFlags(void) >> { >> FILE *fp; >> virBitmap *flags = NULL; >> + g_autofree char *line = NULL; >> + size_t linelen = 0; >> >> if (!(fp = fopen("/proc/cpuinfo", "r"))) >> return NULL; >> >> flags = virBitmapNew(VIR_HOST_VALIDATE_CPU_FLAG_LAST); >> >> - do { >> - char line[1024]; >> + while (getline(&line, &linelen, fp) > 0) { >> char *start; >> g_auto(GStrv) tokens = NULL; >> GStrv next; >> >> - if (!fgets(line, sizeof(line), fp)) >> - break; >> - >> /* The line we're interested in is marked differently depending >> * on the architecture, so check possible prefixes */ >> if (!STRPREFIX(line, "flags") && >> @@ -129,12 +127,6 @@ virBitmap *virHostValidateGetCPUFlags(void) >> !STRPREFIX(line, "facilities")) >> continue; >> >> - /* fgets() includes the trailing newline in the output buffer, >> - * so we need to clean that up ourselves. We can safely access >> - * line[strlen(line) - 1] because the checks above would cause >> - * us to skip empty strings */ >> - line[strlen(line) - 1] = '\0'; >> - > > getline() also includes the trailing newline in the buffer (as long as > there is any newline) and thus we still need to drop it. We just don't > need to call strlen to compute the length. Although I would suggest > using a little bit more robust code instead of describing the existing > code is safe... good point > > if (linelen > 1 && line[linelen - 1] == '\n') > line[linelen - 1] = '\0'; I'm going with virStringTrimOptionalNewline(). Michal
