Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-1747-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Mon, 12 Feb 2024 09:30:47 +0100
From: Mariusz Kozlowski <mk@lab.zgora.pl>
To: "Mariusz Kozlowski" <mk@lab.zgora.pl>
Cc: "Luiz Augusto von Dentz" <luiz.dentz@gmail.com>,
	"linux-bluetooth" <linux-bluetooth@vger.kernel.org>
Message-ID: <18d9c70b2a0.d3e08593351442.8477048035206577660@lab.zgora.pl>
In-Reply-To: <18d356110eb.1120f313b408792.7694888926917426249@lab.zgora.pl>
References: <20240121100328.1200839-1-mk@lab.zgora.pl> <CABBYNZKW8odyLb_BQcXXpjAKjPzHRhXVCbxvcg54VkoeULeHnw@mail.gmail.com> <18d356110eb.1120f313b408792.7694888926917426249@lab.zgora.pl>
Subject: Re: [PATCH BlueZ] btmon-logger: Fix stack corruption
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Importance: Medium
User-Agent: Zoho Mail

Hi Luiz,

---- On Tue, 23 Jan 2024 09:12:50 +0100 Mariusz Kozlowski  wrote ---

 > Hi Luiz,=20
 > =20
 >  > Hi Mariusz,=20
 >  >=20
 >  > On Sun, Jan 21, 2024 at 5:04=E2=80=AFAM Mariusz Koz=C5=82owski mk@lab=
.zgora.pl> wrote:=20
 >  > >=20
 >  > > Version 3 capability masks are 64 bits in size.=20
 >  > > ---=20
 >  > >  tools/btmon-logger.c | 12 ++++++------=20
 >  > >  1 file changed, 6 insertions(+), 6 deletions(-)=20
 >  > >=20
 >  > > diff --git a/tools/btmon-logger.c b/tools/btmon-logger.c=20
 >  > > index a770ad575..1f6db3751 100644=20
 >  > > --- a/tools/btmon-logger.c=20
 >  > > +++ b/tools/btmon-logger.c=20
 >  > > @@ -161,14 +161,14 @@ extern int capset(struct __user_cap_header_st=
ruct *header,=20
 >  > >  static void drop_capabilities(void)=20
 >  > >  {=20
 >  > >         struct __user_cap_header_struct header;=20
 >  > > -       struct __user_cap_data_struct cap;=20
 >  > > +       struct __user_cap_data_struct cap[_LINUX_CAPABILITY_U32S_3]=
;=20
 >  >=20
 >  > Ok, but this doesn't change the field, it makes it an array, or are=
=20
 >  > you talking about the following note:=20
 >  >=20
 >  > Note that 64-bit capabilities use datap[0] and datap[1], whereas=20
 >  > 32-bit capabilities use only datap[0].=20
 >  >=20
 >  > In that case Ive just pointed out to this note to explain why this is=
 needed.=20
 > =20
 > For version 3 caps (64 bit masks) a single struct __user_cap_data_struct=
 is not=20
 > big enough and capget() writes past the end of cap structure on the stac=
k. To=20
 > accomodate version 3 cap masks the cap structure needs to be 2x bigger.=
=20
=20
What is the status of this patch? I don't see it either accepted or rejecte=
d.

 >  > >         unsigned int mask;=20
 >  > >         int err;=20
 >  > >=20
 >  > >         header.version =3D _LINUX_CAPABILITY_VERSION_3;=20
 >  > >         header.pid =3D 0;=20
 >  > >=20
 >  > > -       err =3D capget(&header, &cap);=20
 >  > > +       err =3D capget(&header, cap);=20
 >  > >         if (err) {=20
 >  > >                 perror("Unable to get current capabilities");=20
 >  > >                 return;=20
 >  > > @@ -177,11 +177,11 @@ static void drop_capabilities(void)=20
 >  > >         /* not needed anymore since monitor socket is already open =
*/=20
 >  > >         mask =3D ~CAP_TO_MASK(CAP_NET_RAW);=20
 >  > >=20
 >  > > -       cap.effective &=3D mask;=20
 >  > > -       cap.permitted &=3D mask;=20
 >  > > -       cap.inheritable &=3D mask;=20
 >  > > +       cap[0].effective &=3D mask;=20
 >  > > +       cap[0].permitted &=3D mask;=20
 >  > > +       cap[0].inheritable &=3D mask;=20
 >  > >=20
 >  > > -       err =3D capset(&header, &cap);=20
 >  > > +       err =3D capset(&header, cap);=20
 >  > >         if (err)=20
 >  > >                 perror("Failed to set capabilities");=20
 >  > >  }=20

Thanks,
Mariusz