| Message ID | 5969bb991adedb03c6ae93e051fd2a00d293cf25.1627513670.git.dxu@dxuuu.xyz (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf] bpf: Do not close un-owned FD 0 on errors | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for bpf |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | fail | 1 blamed authors not CCed: ast@kernel.org; 8 maintainers not CCed: netdev@vger.kernel.org yhs@fb.com kpsingh@kernel.org daniel@iogearbox.net kafai@fb.com ast@kernel.org john.fastabend@gmail.com songliubraving@fb.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 15 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On 7/28/21 4:09 PM, Daniel Xu wrote: > Before this patch, btf_new() was liable to close an arbitrary FD 0 if > BTF parsing failed. This was because: > > * btf->fd was initialized to 0 through the calloc() > * btf__free() (in the `done` label) closed any FDs >= 0 > * btf->fd is left at 0 if parsing fails > > This issue was discovered on a system using libbpf v0.3 (without > BTF_KIND_FLOAT support) but with a kernel that had BTF_KIND_FLOAT types > in BTF. Thus, parsing fails. > > While this patch technically doesn't fix any issues b/c upstream libbpf > has BTF_KIND_FLOAT support, it'll help prevent issues in the future if > more BTF types are added. It also allow the fix to be backported to > older libbpf's. > > Fixes: 3289959b97ca ("libbpf: Support BTF loading and raw data output in both endianness") > Signed-off-by: Daniel Xu <dxu@dxuuu.xyz> Acked-by: Yonghong Song <yhs@fb.com>
On Wed, Jul 28, 2021 at 4:09 PM Daniel Xu <dxu@dxuuu.xyz> wrote: > > Before this patch, btf_new() was liable to close an arbitrary FD 0 if > BTF parsing failed. This was because: > > * btf->fd was initialized to 0 through the calloc() > * btf__free() (in the `done` label) closed any FDs >= 0 > * btf->fd is left at 0 if parsing fails > > This issue was discovered on a system using libbpf v0.3 (without > BTF_KIND_FLOAT support) but with a kernel that had BTF_KIND_FLOAT types > in BTF. Thus, parsing fails. > > While this patch technically doesn't fix any issues b/c upstream libbpf > has BTF_KIND_FLOAT support, it'll help prevent issues in the future if > more BTF types are added. It also allow the fix to be backported to > older libbpf's. > > Fixes: 3289959b97ca ("libbpf: Support BTF loading and raw data output in both endianness") > Signed-off-by: Daniel Xu <dxu@dxuuu.xyz> > --- Thanks! Applied to bpf. We should bite a bullet and make sure that libbpf itself never uses/allows FD 0 internally (by, say, dup()'ing FD 0, if we happen to get it) and get rid of the -1 special initializers. > tools/lib/bpf/btf.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c > index b46760b93bb4..7ff3d5ce44f9 100644 > --- a/tools/lib/bpf/btf.c > +++ b/tools/lib/bpf/btf.c > @@ -804,6 +804,7 @@ static struct btf *btf_new(const void *data, __u32 size, struct btf *base_btf) > btf->nr_types = 0; > btf->start_id = 1; > btf->start_str_off = 0; > + btf->fd = -1; > > if (base_btf) { > btf->base_btf = base_btf; > @@ -832,8 +833,6 @@ static struct btf *btf_new(const void *data, __u32 size, struct btf *base_btf) > if (err) > goto done; > > - btf->fd = -1; > - > done: > if (err) { > btf__free(btf); > -- > 2.31.1 >
diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c index b46760b93bb4..7ff3d5ce44f9 100644 --- a/tools/lib/bpf/btf.c +++ b/tools/lib/bpf/btf.c @@ -804,6 +804,7 @@ static struct btf *btf_new(const void *data, __u32 size, struct btf *base_btf) btf->nr_types = 0; btf->start_id = 1; btf->start_str_off = 0; + btf->fd = -1; if (base_btf) { btf->base_btf = base_btf; @@ -832,8 +833,6 @@ static struct btf *btf_new(const void *data, __u32 size, struct btf *base_btf) if (err) goto done; - btf->fd = -1; - done: if (err) { btf__free(btf);
Before this patch, btf_new() was liable to close an arbitrary FD 0 if BTF parsing failed. This was because: * btf->fd was initialized to 0 through the calloc() * btf__free() (in the `done` label) closed any FDs >= 0 * btf->fd is left at 0 if parsing fails This issue was discovered on a system using libbpf v0.3 (without BTF_KIND_FLOAT support) but with a kernel that had BTF_KIND_FLOAT types in BTF. Thus, parsing fails. While this patch technically doesn't fix any issues b/c upstream libbpf has BTF_KIND_FLOAT support, it'll help prevent issues in the future if more BTF types are added. It also allow the fix to be backported to older libbpf's. Fixes: 3289959b97ca ("libbpf: Support BTF loading and raw data output in both endianness") Signed-off-by: Daniel Xu <dxu@dxuuu.xyz> --- tools/lib/bpf/btf.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)