| Message ID | 20230723075452.3711158-1-linma@zju.edu.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | [v1] bpf: Add length check for SK_DIAG_BPF_STORAGE_REQ_MAP_FD parsing | expand |
On Sun, 23 Jul 2023 15:54:52 +0800 Lin Ma wrote: > diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c > index d4172534dfa8..6f1afbb394a6 100644 > --- a/net/core/bpf_sk_storage.c > +++ b/net/core/bpf_sk_storage.c > @@ -511,6 +511,11 @@ bpf_sk_storage_diag_alloc(const struct nlattr *nla_stgs) > if (nla_type(nla) != SK_DIAG_BPF_STORAGE_REQ_MAP_FD) > continue; > > + if (nla_len(nla) < sizeof(map_fd)) { > + err = -EINVAL; > + goto err_free; > + } You can move this check earlier, when the attributes are getting counted. That way we can avoid the alloc/free on error.
Hi Jakub, > > You can move this check earlier, when the attributes are getting > counted. That way we can avoid the alloc/free on error. Good point, will fix that and prepare another patch Thanks Lin
diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c index d4172534dfa8..6f1afbb394a6 100644 --- a/net/core/bpf_sk_storage.c +++ b/net/core/bpf_sk_storage.c @@ -511,6 +511,11 @@ bpf_sk_storage_diag_alloc(const struct nlattr *nla_stgs) if (nla_type(nla) != SK_DIAG_BPF_STORAGE_REQ_MAP_FD) continue; + if (nla_len(nla) < sizeof(map_fd)) { + err = -EINVAL; + goto err_free; + } + map_fd = nla_get_u32(nla); map = bpf_map_get(map_fd); if (IS_ERR(map)) {
The nla_for_each_nested parsing in function bpf_sk_storage_diag_alloc does not check the length of the nested attribute. This can lead to an out-of-attribute read and allow a malformed nlattr (e.g., length 0) to be viewed as a 4 byte integer. This patch adds additional check before the execution of nla_get_u32 to make sure the attribute has enough length. Fixes: 1ed4d92458a9 ("bpf: INET_DIAG support in bpf_sk_storage") Signed-off-by: Lin Ma <linma@zju.edu.cn> --- net/core/bpf_sk_storage.c | 5 +++++ 1 file changed, 5 insertions(+)