| Message ID | 20210512102529.122753-2-omosnace@redhat.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | Bump testsuite CI image to F34 | expand |
Am Mi., 12. Mai 2021 um 12:25 Uhr schrieb Ondrej Mosnacek <omosnace@redhat.com>: > > Building libselinux with GCC 11.1.1 produces the following warning: > ``` > In file included from label_file.c:24: > In function ‘store_stem’, > inlined from ‘load_mmap’ at label_file.c:277:12, > inlined from ‘process_file’ at label_file.c:551:5: > label_file.h:289:25: error: ‘free’ called on pointer ‘*mmap_area.next_addr’ with nonzero offset 4 [-Werror=free-nonheap-object] > 289 | free(buf); > | ^~~~~~~~~ > ``` > > Indeed, in this case the pointer shouldn't be freed as it comes from > mmap. Fix this by adding a from_mmap parameter to store_stem() instead > of overriding the saved_data::from_mmap value in load_mmap(). > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> See https://patchwork.kernel.org/project/selinux/patch/20210503175350.55954-17-cgzones@googlemail.com/ for an alternative.
Christian Göttsche <cgzones@googlemail.com> writes: > Am Mi., 12. Mai 2021 um 12:25 Uhr schrieb Ondrej Mosnacek <omosnace@redhat.com>: >> >> Building libselinux with GCC 11.1.1 produces the following warning: >> ``` >> In file included from label_file.c:24: >> In function ‘store_stem’, >> inlined from ‘load_mmap’ at label_file.c:277:12, >> inlined from ‘process_file’ at label_file.c:551:5: >> label_file.h:289:25: error: ‘free’ called on pointer ‘*mmap_area.next_addr’ with nonzero offset 4 [-Werror=free-nonheap-object] >> 289 | free(buf); >> | ^~~~~~~~~ >> ``` >> >> Indeed, in this case the pointer shouldn't be freed as it comes from >> mmap. Fix this by adding a from_mmap parameter to store_stem() instead >> of overriding the saved_data::from_mmap value in load_mmap(). >> >> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > > See https://patchwork.kernel.org/project/selinux/patch/20210503175350.55954-17-cgzones@googlemail.com/ > for an alternative. https://patchwork.kernel.org/project/selinux/patch/20210503175350.55954-17-cgzones@googlemail.com/ - Accepted https://patchwork.kernel.org/project/selinux/patch/20210512102529.122753-2-omosnace@redhat.com/ - Rejected Thanks to both!
diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c index cfce23e0..199ae66b 100644 --- a/libselinux/src/label_file.c +++ b/libselinux/src/label_file.c @@ -274,12 +274,11 @@ end_arch_check: /* store the mapping between old and new */ newid = find_stem(data, buf, stem_len); if (newid < 0) { - newid = store_stem(data, buf, stem_len); + newid = store_stem(data, buf, stem_len, true); if (newid < 0) { rc = newid; goto out; } - data->stem_arr[newid].from_mmap = 1; } stem_map[i] = newid; } diff --git a/libselinux/src/label_file.h b/libselinux/src/label_file.h index baed3341..66e5721f 100644 --- a/libselinux/src/label_file.h +++ b/libselinux/src/label_file.h @@ -62,7 +62,7 @@ struct spec { struct stem { char *buf; int len; - char from_mmap; + bool from_mmap; }; /* Where we map the file in during selabel_open() */ @@ -276,7 +276,8 @@ static inline int find_stem(struct saved_data *data, const char *buf, } /* returns the index of the new stored object */ -static inline int store_stem(struct saved_data *data, char *buf, int stem_len) +static inline int store_stem(struct saved_data *data, char *buf, int stem_len, + bool from_mmap) { int num = data->num_stems; @@ -286,7 +287,8 @@ static inline int store_stem(struct saved_data *data, char *buf, int stem_len) tmp_arr = realloc(data->stem_arr, sizeof(*tmp_arr) * alloc_stems); if (!tmp_arr) { - free(buf); + if (!from_mmap) + free(buf); return -1; } data->alloc_stems = alloc_stems; @@ -294,7 +296,7 @@ static inline int store_stem(struct saved_data *data, char *buf, int stem_len) } data->stem_arr[num].len = stem_len; data->stem_arr[num].buf = buf; - data->stem_arr[num].from_mmap = 0; + data->stem_arr[num].from_mmap = from_mmap; data->num_stems++; return num; @@ -321,7 +323,7 @@ static inline int find_stem_from_spec(struct saved_data *data, const char *buf) if (!stem) return -1; - return store_stem(data, stem, stem_len); + return store_stem(data, stem, stem_len, false); } /* This will always check for buffer over-runs and either read the next entry
Building libselinux with GCC 11.1.1 produces the following warning: ``` In file included from label_file.c:24: In function ‘store_stem’, inlined from ‘load_mmap’ at label_file.c:277:12, inlined from ‘process_file’ at label_file.c:551:5: label_file.h:289:25: error: ‘free’ called on pointer ‘*mmap_area.next_addr’ with nonzero offset 4 [-Werror=free-nonheap-object] 289 | free(buf); | ^~~~~~~~~ ``` Indeed, in this case the pointer shouldn't be freed as it comes from mmap. Fix this by adding a from_mmap parameter to store_stem() instead of overriding the saved_data::from_mmap value in load_mmap(). Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> --- libselinux/src/label_file.c | 3 +-- libselinux/src/label_file.h | 12 +++++++----- 2 files changed, 8 insertions(+), 7 deletions(-)