Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 10 Aug 2023 10:22:47 -0700
From:   Jakub Kicinski <kuba@kernel.org>
To:     Jijie Shao <shaojijie@huawei.com>,
        Kees Cook <keescook@chromium.org>
Cc:     Leon Romanovsky <leon@kernel.org>, <yisen.zhuang@huawei.com>,
        <salil.mehta@huawei.com>, <davem@davemloft.net>,
        <edumazet@google.com>, <pabeni@redhat.com>,
        <shenjian15@huawei.com>, <wangjie125@huawei.com>,
        <liuyonglong@huawei.com>, <chenhao418@huawei.com>,
        <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <stable@vger.kernel.org>
Subject: Re: [PATCH net] net: hns3: fix strscpy causing content truncation
 issue
Message-ID: <20230810102247.699ddc14@kernel.org>
In-Reply-To: <7c44c161-9c86-8c60-f031-6d77d6c28c20@huawei.com>
References: <20230809020902.1941471-1-shaojijie@huawei.com>
        <20230809070302.GR94631@unreal>
        <7c44c161-9c86-8c60-f031-6d77d6c28c20@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, 10 Aug 2023 15:45:50 +0800 Jijie Shao wrote:
> on 2023/8/9 15:03, Leon Romanovsky wrote:
> > On Wed, Aug 09, 2023 at 10:09:02AM +0800, Jijie Shao wrote: =20
> >> From: Hao Chen <chenhao418@huawei.com>
> >>
> >> hns3_dbg_fill_content()/hclge_dbg_fill_content() is aim to integrate s=
ome
> >> items to a string for content, and we add '\n' and '\0' in the last
> >> two bytes of content.
> >>
> >> strscpy() will add '\0' in the last byte of destination buffer(one of
> >> items), it result in finishing content print ahead of schedule and some
> >> dump content truncation.
> >>
> >> One Error log shows as below:
> >> cat mac_list/uc
> >> UC MAC_LIST:
> >>
> >> Expected:
> >> UC MAC_LIST:
> >> FUNC_ID  MAC_ADDR            STATE
> >> pf       00:2b:19:05:03:00   ACTIVE
> >>
> >> The destination buffer is length-bounded and not required to be
> >> NUL-terminated, so just change strscpy() to memcpy() to fix it. =20
> > I think that you should change to strtomem() and not use plain memcpy().
> >
> > Thanks =20
>=20
> Hi:
>=20
> We tried to replace memcpy with strtomem, but errors was reported during=
=20
> compilation:
> /kernel/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c: In=20
> function =E2=80=98hclge_dbg_fill_content.part.0=E2=80=99:
> /kernel/include/linux/compiler_types.h:397:38: error: call to=20
> =E2=80=98__compiletime_assert_519=E2=80=99 declared with attribute error:=
 BUILD_BUG_ON=20
> failed: !__builtin_constant_p(_dest_len) || _dest_len =3D=3D (size_t)-1
>  =C2=A0 397 |=C2=A0 _compiletime_assert(condition, msg, __compiletime_ass=
ert_,=20
> __COUNTER__)
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ^
> /kernel/include/linux/compiler_types.h:378:4: note: in definition of=20
> macro =E2=80=98__compiletime_assert=E2=80=99
>  =C2=A0 378 |=C2=A0=C2=A0=C2=A0 prefix ## suffix();=C2=A0=C2=A0=C2=A0 \
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0=C2=A0=C2=A0 ^~~~~~
> /kernel/include/linux/compiler_types.h:397:2: note: in expansion of=20
> macro =E2=80=98_compiletime_assert=E2=80=99
>  =C2=A0 397 |=C2=A0 _compiletime_assert(condition, msg, __compiletime_ass=
ert_,=20
> __COUNTER__)
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0 ^~~~~~~~~~~~~~~~~~~
> /kernel/include/linux/build_bug.h:39:37: note: in expansion of macro=20
> =E2=80=98compiletime_assert=E2=80=99
>  =C2=A0=C2=A0 39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert=
(!(cond),=20
> msg)
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0 ^~~~~~~~~~~~~~~~~~
> /kernel/include/linux/build_bug.h:50:2: note: in expansion of macro=20
> =E2=80=98BUILD_BUG_ON_MSG=E2=80=99
>  =C2=A0=C2=A0 50 |=C2=A0 BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed=
: " #condition)
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0 ^~~~~~~~~~~~~~~~
> /kernel/include/linux/string.h:302:2: note: in expansion of macro=20
> =E2=80=98BUILD_BUG_ON=E2=80=99
>  =C2=A0 302 |=C2=A0 BUILD_BUG_ON(!__builtin_constant_p(_dest_len) ||=C2=
=A0 \
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0 ^~~~~~~~~~~~
> /kernel/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:115:4:=
=20
> note: in expansion of macro =E2=80=98strtomem=E2=80=99
>  =C2=A0 115 |=C2=A0=C2=A0=C2=A0 strtomem(pos, result[i]);
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0=C2=A0=C2=A0 ^~~~~~~~
>=20
> In the strtomem macro, __builtin_object_size is used to calculate the=20
> _dest_len.
> We tried to print the _dest_len directly, and the result was -1.
> How can we solve this?

Let's add Kees in case he has a immediate recommendation on use of
strtomem() vs memcpy() for this case..