1. Patchew
  2. Xen
  3. View series

[PATCH] flask: fix gcov build with gcc14+

Jan Beulich posted 1 patch 4 weeks, 1 day ago
Failed in applying to current master (apply log)
[PATCH] flask: fix gcov build with gcc14+
Posted by Jan Beulich 4 weeks, 1 day ago 
Gcc's "threading" of conditionals can lead to undue warnings, as reported
in e.g. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116519 (no matter
that the overall situation is different there). While my gcc15 complains
("buf[2] may be used uninitialized in this function") about only two of
the three instances (not about the one in type_read()), adjust all three
to be on the safe side.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
While auditing uses of next_entry(), I noticed POLICYDB_VERSION_ROLETRANS
dependent ones in policydb_read(): How come the 4th slot isn't used at all
there (not even checked for being e.g. zero, i.e. holding no useful data)?
Then again other instances can be found where data is read but outright
ignored.

--- a/xen/xsm/flask/ss/policydb.c
+++ b/xen/xsm/flask/ss/policydb.c
@@ -1271,7 +1271,10 @@ static int cf_check role_read(struct pol
     if ( ver >= POLICYDB_VERSION_BOUNDARY )
         rc = next_entry(buf, fp, sizeof(buf[0]) * 3);
     else
+    {
         rc = next_entry(buf, fp, sizeof(buf[0]) * 2);
+        buf[2] = cpu_to_le32(0); /* gcc14 onwards */
+    }
 
     if ( rc < 0 )
         goto bad;
@@ -1342,7 +1345,10 @@ static int cf_check type_read(struct pol
     if ( ver >= POLICYDB_VERSION_BOUNDARY )
         rc = next_entry(buf, fp, sizeof(buf[0]) * 4);
     else
+    {
         rc = next_entry(buf, fp, sizeof(buf[0]) * 3);
+        buf[3] = cpu_to_le32(0); /* gcc14 onwards */
+    }
 
     if ( rc < 0 )
         goto bad;
@@ -1436,7 +1442,10 @@ static int cf_check user_read(struct pol
     if ( ver >= POLICYDB_VERSION_BOUNDARY )
         rc = next_entry(buf, fp, sizeof(buf[0]) * 3);
     else
+    {
         rc = next_entry(buf, fp, sizeof(buf[0]) * 2);
+        buf[2] = cpu_to_le32(0); /* gcc14 onwards */
+    }
 
     if ( rc < 0 )
         goto bad;
Ping: [PATCH] flask: fix gcov build with gcc14+
Posted by Jan Beulich 2 weeks, 4 days ago 
Daniel,

On 08.01.2026 10:18, Jan Beulich wrote:
> Gcc's "threading" of conditionals can lead to undue warnings, as reported
> in e.g. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116519 (no matter
> that the overall situation is different there). While my gcc15 complains
> ("buf[2] may be used uninitialized in this function") about only two of
> the three instances (not about the one in type_read()), adjust all three
> to be on the safe side.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

any chance of an ack (or otherwise)?

Thanks, Jan

> ---
> While auditing uses of next_entry(), I noticed POLICYDB_VERSION_ROLETRANS
> dependent ones in policydb_read(): How come the 4th slot isn't used at all
> there (not even checked for being e.g. zero, i.e. holding no useful data)?
> Then again other instances can be found where data is read but outright
> ignored.
> 
> --- a/xen/xsm/flask/ss/policydb.c
> +++ b/xen/xsm/flask/ss/policydb.c
> @@ -1271,7 +1271,10 @@ static int cf_check role_read(struct pol
>      if ( ver >= POLICYDB_VERSION_BOUNDARY )
>          rc = next_entry(buf, fp, sizeof(buf[0]) * 3);
>      else
> +    {
>          rc = next_entry(buf, fp, sizeof(buf[0]) * 2);
> +        buf[2] = cpu_to_le32(0); /* gcc14 onwards */
> +    }
>  
>      if ( rc < 0 )
>          goto bad;
> @@ -1342,7 +1345,10 @@ static int cf_check type_read(struct pol
>      if ( ver >= POLICYDB_VERSION_BOUNDARY )
>          rc = next_entry(buf, fp, sizeof(buf[0]) * 4);
>      else
> +    {
>          rc = next_entry(buf, fp, sizeof(buf[0]) * 3);
> +        buf[3] = cpu_to_le32(0); /* gcc14 onwards */
> +    }
>  
>      if ( rc < 0 )
>          goto bad;
> @@ -1436,7 +1442,10 @@ static int cf_check user_read(struct pol
>      if ( ver >= POLICYDB_VERSION_BOUNDARY )
>          rc = next_entry(buf, fp, sizeof(buf[0]) * 3);
>      else
> +    {
>          rc = next_entry(buf, fp, sizeof(buf[0]) * 2);
> +        buf[2] = cpu_to_le32(0); /* gcc14 onwards */
> +    }
>  
>      if ( rc < 0 )
>          goto bad;
Re: Ping: [PATCH] flask: fix gcov build with gcc14+
Posted by Daniel P. Smith 2 weeks, 2 days ago 
Jan, 

Apologies, I've been on travel for the last two weeks and I wasn't comfortable acking this with just a read of the diff. The thing that bothers me that I want to understand better is why only after the else does it worry about null terminated. Additionally, stepping back, a casual reader of the code is going to wonder why only after some reads into the buffer does it need a null while others do not. I think most people would find that as a red flag that an underlying issue is getting papers papered over. I will be back from travel this weekend and I will sit down and review with more context. 

V/r,
DPS 

On January 19, 2026 8:50:02 AM CST, Jan Beulich <jbeulich@suse.com> wrote:
>Daniel,
>
>On 08.01.2026 10:18, Jan Beulich wrote:
>> Gcc's "threading" of conditionals can lead to undue warnings, as reported
>> in e.g. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116519 (no matter
>> that the overall situation is different there). While my gcc15 complains
>> ("buf[2] may be used uninitialized in this function") about only two of
>> the three instances (not about the one in type_read()), adjust all three
>> to be on the safe side.
>> 
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
>any chance of an ack (or otherwise)?
>
>Thanks, Jan
>
>> ---
>> While auditing uses of next_entry(), I noticed POLICYDB_VERSION_ROLETRANS
>> dependent ones in policydb_read(): How come the 4th slot isn't used at all
>> there (not even checked for being e.g. zero, i.e. holding no useful data)?
>> Then again other instances can be found where data is read but outright
>> ignored.
>> 
>> --- a/xen/xsm/flask/ss/policydb.c
>> +++ b/xen/xsm/flask/ss/policydb.c
>> @@ -1271,7 +1271,10 @@ static int cf_check role_read(struct pol
>>      if ( ver >= POLICYDB_VERSION_BOUNDARY )
>>          rc = next_entry(buf, fp, sizeof(buf[0]) * 3);
>>      else
>> +    {
>>          rc = next_entry(buf, fp, sizeof(buf[0]) * 2);
>> +        buf[2] = cpu_to_le32(0); /* gcc14 onwards */
>> +    }
>>  
>>      if ( rc < 0 )
>>          goto bad;
>> @@ -1342,7 +1345,10 @@ static int cf_check type_read(struct pol
>>      if ( ver >= POLICYDB_VERSION_BOUNDARY )
>>          rc = next_entry(buf, fp, sizeof(buf[0]) * 4);
>>      else
>> +    {
>>          rc = next_entry(buf, fp, sizeof(buf[0]) * 3);
>> +        buf[3] = cpu_to_le32(0); /* gcc14 onwards */
>> +    }
>>  
>>      if ( rc < 0 )
>>          goto bad;
>> @@ -1436,7 +1442,10 @@ static int cf_check user_read(struct pol
>>      if ( ver >= POLICYDB_VERSION_BOUNDARY )
>>          rc = next_entry(buf, fp, sizeof(buf[0]) * 3);
>>      else
>> +    {
>>          rc = next_entry(buf, fp, sizeof(buf[0]) * 2);
>> +        buf[2] = cpu_to_le32(0); /* gcc14 onwards */
>> +    }
>>  
>>      if ( rc < 0 )
>>          goto bad;
>
Re: Ping: [PATCH] flask: fix gcov build with gcc14+
Posted by Jan Beulich 2 weeks, 2 days ago 
On 21.01.2026 14:14, Daniel P. Smith wrote:
> Apologies, I've been on travel for the last two weeks and I wasn't comfortable acking this with just a read of the diff. The thing that bothers me that I want to understand better is why only after the else does it worry about null terminated. Additionally, stepping back, a casual reader of the code is going to wonder why only after some reads into the buffer does it need a null while others do not.

I'm curious to know of an example or two which you refer to here, as ...

> I think most people would find that as a red flag that an underlying issue is getting papers papered over. I will be back from travel this weekend and I will sit down and review with more context. 
> 
> V/r,
> DPS 
> 
> On January 19, 2026 8:50:02 AM CST, Jan Beulich <jbeulich@suse.com> wrote:
>> Daniel,
>>
>> On 08.01.2026 10:18, Jan Beulich wrote:
>>> Gcc's "threading" of conditionals can lead to undue warnings, as reported
>>> in e.g. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116519 (no matter
>>> that the overall situation is different there). While my gcc15 complains
>>> ("buf[2] may be used uninitialized in this function") about only two of
>>> the three instances (not about the one in type_read()), adjust all three
>>> to be on the safe side.

... I've already extended the change to cover all three similar patterns, no
matter that only two triggered a warning.

Jan
Re: Ping: [PATCH] flask: fix gcov build with gcc14+
Posted by Daniel P. Smith 1 week, 5 days ago 
On 1/21/26 8:35 AM, Jan Beulich wrote:
> On 21.01.2026 14:14, Daniel P. Smith wrote:
>> Apologies, I've been on travel for the last two weeks and I wasn't comfortable acking this with just a read of the diff. The thing that bothers me that I want to understand better is why only after the else does it worry about null terminated. Additionally, stepping back, a casual reader of the code is going to wonder why only after some reads into the buffer does it need a null while others do not.
> 
> I'm curious to know of an example or two which you refer to here, as ...

The diff does not show the size of buf, and I was not going to assume 
the size. Had the explanation said something to the extent that when an 
array of size N is used as a buffer that is filled with N-1 (or less?) 
bytes under a conditional, gcc15 complains about the trailing bytes 
being uninitialized. Without the context about the buffer size, it was 
not clear to me from your explanation, or the link, as to why only under 
the else cases need setting the value on your last byte and not under 
the initial if.

Now that I have better context, my initial concerns were not valid and 
am more comfortable providing the ack.

Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>

>> I think most people would find that as a red flag that an underlying issue is getting papers papered over. I will be back from travel this weekend and I will sit down and review with more context.
>>
>> V/r,
>> DPS
>>
>> On January 19, 2026 8:50:02 AM CST, Jan Beulich <jbeulich@suse.com> wrote:
>>> Daniel,
>>>
>>> On 08.01.2026 10:18, Jan Beulich wrote:
>>>> Gcc's "threading" of conditionals can lead to undue warnings, as reported
>>>> in e.g. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116519 (no matter
>>>> that the overall situation is different there). While my gcc15 complains
>>>> ("buf[2] may be used uninitialized in this function") about only two of
>>>> the three instances (not about the one in type_read()), adjust all three
>>>> to be on the safe side.
> 
> ... I've already extended the change to cover all three similar patterns, no
> matter that only two triggered a warning.
> 
> Jan

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.