Re: [PATCH] Fix listxattr-related races and stack overflows

[Paul Eggert]

On 2025-06-04 04:27, Andreas Grünbacher wrote:
> Am Sa., 31. Mai 2025 um 17:49 Uhr schrieb Paul Eggert <eggert@cs.ucla.edu>:

> Thanks, I've applied this with the following additional
> simplicifaction in both functions:

Thanks, looks good.

I looked into doing a fix for the fgetxattr and lgetxattr calls too: 
namely, don't trust the sizes that you get with NULL arg since there 
could be a race. While I was at it I noticed some other issues in the 
neighborhood and propose the attached further patches to fix the issues 
that I found.

* The patch I sent you had an off-by-one error in allocation. Sorry 
about that. Fixed in patch 0001.

* In penance I looked for other possible errors, and found an unlikely 
unsigned overflow that could cause misbehavior. Fixed in patch 0002.

* The off-by-one-error fixed in patch 0001 got me to thinking that the 
memory allocation code doesn't need to be so complicated, as the 
complexity is present only for optimizing a case that never happens in 
practice. I refactored in patch 0003 and simplified memory allocation in 
patch 0004.

* Patch 0005 implements the "something similar" optimization I mentioned 
above. If the file has N attributes, this typically saves N syscalls, 
plus it avoids some races.

* The strchr call can be simplified to strlen. No big deal either way, 
but it's easier to read (at least for me), and arguably a bit faster. 
This is patch 0006.

These patches preserve what appears to be defensive code that doesn't 
trust flistxattr/llistxattr to return a '\0'-terminated buffer when 
these syscalls return a positive value. Patch 0005 adds a comment about 
that. Has this kernel bug been observed in practice? If so, that comment 
could be improved to mention if/when the bug has been fixed.

0001-Fix-off-by-one-allocation-error-in-recent-patch.patch
Description: Text Data

0002-Fix-unlikely-unsigned-overflow-bug-in-attr_copy_f.patch
Description: Text Data

0003-Refactor-attr_copy_-for-future-changes.patch
Description: Text Data

0004-Simplify-almost-never-used-reallocation-code.patch
Description: Text Data

0005-Fix-lgetxattr-related-races.patch
Description: Text Data

0006-strchr-strlen-in-attr_copy_f.patch
Description: Text Data