lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEf4BzZVYO42kDcmNqorLfwJcMcN7fyTLdp2GWbGfV5akP12GQ@mail.gmail.com>
Date:   Thu, 13 Oct 2022 08:47:55 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Xu Kuohai <xukuohai@...weicloud.com>
Cc:     bpf@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-kselftest@...r.kernel.org, netdev@...r.kernel.org,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <martin.lau@...ux.dev>,
        Song Liu <song@...nel.org>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        Stanislav Fomichev <sdf@...gle.com>,
        Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
        Mykola Lysenko <mykolal@...com>, Shuah Khan <shuah@...nel.org>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        Kumar Kartikeya Dwivedi <memxor@...il.com>,
        Alan Maguire <alan.maguire@...cle.com>,
        Delyan Kratunov <delyank@...com>,
        Lorenzo Bianconi <lorenzo@...nel.org>
Subject: Re: [PATCH bpf-next v4 2/6] libbpf: Fix memory leak in parse_usdt_arg()

On Tue, Oct 11, 2022 at 4:43 AM Xu Kuohai <xukuohai@...weicloud.com> wrote:
>
> From: Xu Kuohai <xukuohai@...wei.com>
>
> In the arm64 version of parse_usdt_arg(), when sscanf returns 2, reg_name
> is allocated but not freed. Fix it.
>
> Fixes: 0f8619929c57 ("libbpf: Usdt aarch64 arg parsing support")
> Signed-off-by: Xu Kuohai <xukuohai@...wei.com>
> ---
>  tools/lib/bpf/usdt.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c
> index e83b497c2245..49f3c3b7f609 100644
> --- a/tools/lib/bpf/usdt.c
> +++ b/tools/lib/bpf/usdt.c
> @@ -1348,25 +1348,23 @@ static int calc_pt_regs_off(const char *reg_name)
>
>  static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec *arg)
>  {
> -       char *reg_name = NULL;
> +       char reg_name[16];
>         int arg_sz, len, reg_off;
>         long off;
>
> -       if (sscanf(arg_str, " %d @ \[ %m[a-z0-9], %ld ] %n", &arg_sz, &reg_name, &off, &len) == 3) {
> +       if (sscanf(arg_str, " %d @ \[ %15[a-z0-9], %ld ] %n", &arg_sz, reg_name, &off, &len) == 3) {

It would be nice to do the same change for other architectures where
it makes sense and avoid having to deal with unnecessary memory
allocations. Please send follow up patches with similar changes for
other implementations of parse_usdt_arg. Thanks.


>                 /* Memory dereference case, e.g., -4@[sp, 96] */
>                 arg->arg_type = USDT_ARG_REG_DEREF;
>                 arg->val_off = off;
>                 reg_off = calc_pt_regs_off(reg_name);
> -               free(reg_name);
>                 if (reg_off < 0)
>                         return reg_off;
>                 arg->reg_off = reg_off;
> -       } else if (sscanf(arg_str, " %d @ \[ %m[a-z0-9] ] %n", &arg_sz, &reg_name, &len) == 2) {
> +       } else if (sscanf(arg_str, " %d @ \[ %15[a-z0-9] ] %n", &arg_sz, reg_name, &len) == 2) {
>                 /* Memory dereference case, e.g., -4@[sp] */
>                 arg->arg_type = USDT_ARG_REG_DEREF;
>                 arg->val_off = 0;
>                 reg_off = calc_pt_regs_off(reg_name);
> -               free(reg_name);
>                 if (reg_off < 0)
>                         return reg_off;
>                 arg->reg_off = reg_off;
> @@ -1375,12 +1373,11 @@ static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec
>                 arg->arg_type = USDT_ARG_CONST;
>                 arg->val_off = off;
>                 arg->reg_off = 0;
> -       } else if (sscanf(arg_str, " %d @ %m[a-z0-9] %n", &arg_sz, &reg_name, &len) == 2) {
> +       } else if (sscanf(arg_str, " %d @ %15[a-z0-9] %n", &arg_sz, reg_name, &len) == 2) {
>                 /* Register read case, e.g., -8@x4 */
>                 arg->arg_type = USDT_ARG_REG;
>                 arg->val_off = 0;
>                 reg_off = calc_pt_regs_off(reg_name);
> -               free(reg_name);
>                 if (reg_off < 0)
>                         return reg_off;
>                 arg->reg_off = reg_off;
> --
> 2.30.2
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ