Cydox wrote: I slightly changed my mind. Was gonna write that gcc is definitely right, now I'm more at a 50/50. I got two arguments, one from a standards perspective, the other from practical/safety perspective.
## Standards argument: When you do ```C acl = malloc(sizeof(struct posix_acl) + sizeof(struct posix_acl_entry) * 1); acl.a_count = 1; ``` The size of the object that got created there is in fact 40 (feel free to correct me here). The description of `malloc` in the standard says: "The malloc function allocates space for an object whose size is specified by size and whose value is indeterminate." But you could also do ```C acl = malloc(offsetof(struct posix_acl, a_entries) + sizeof(struct posix_acl_entry) * 1); ``` in which case the size of that object is 36. For __bdos we don't know which situation we're in though. ## Practical/Safety argument: It comes down to these 4 cases. A ``` struct posix_acl *acl = malloc(sizeof(struct posix_acl) + sizeof(struct posix_acl_entry) * 1); acl.a_count = 1; memcpy(somewhere, acl, sizeof(struct posix_acl) + sizeof(struct posix_acl_entry) * 1) ``` B ``` struct posix_acl *acl = malloc(sizeof(struct posix_acl) + sizeof(struct posix_acl_entry) * 1); acl.a_count = 1; memcpy(somewhere, acl, offsetof(struct posix_acl, a_entries) + sizeof(struct posix_acl_entry) * 1) ``` C ``` struct posix_acl *acl = malloc(offsetof(struct posix_acl, a_entries) + sizeof(struct posix_acl_entry) * 1); acl.a_count = 1; memcpy(somewhere, acl, sizeof(struct posix_acl) + sizeof(struct posix_acl_entry) * 1) ``` D ``` struct posix_acl *acl = malloc(offsetof(struct posix_acl, a_entries) + sizeof(struct posix_acl_entry) * 1); acl.a_count = 1; memcpy(somewhere, acl, offsetof(struct posix_acl, a_entries) + sizeof(struct posix_acl_entry) * 1) ``` Only C is undefined behavior. With gcc's behavior none of these cases fail the bounds-check, but C is undefined behavior. With clang's behvaior A and C fail the bounds-check, even though A is perfectly safe. So it comes down to whether you want false positives or false negatives. Sticking to the current behavior will lead to random crashes of otherwise safe code that might only trigger occasionally. Code like this will probably lurk for quite a while, especially because gcc has different behavior. Changing the behavior on the other hand will mean not all cases of C might be caught. Which is the correct way to go here is neither obvious nor my call to make. I think we could find a way to scan the kernel for all the A cases and convert them to B or D. I would like to know if any of you disagree or agree with the correct size of the object from the standards perspective. https://github.com/llvm/llvm-project/pull/111015 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
