Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-1250-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Tue, 23 Jan 2024 09:12:50 +0100
From: Mariusz Kozlowski <mk@lab.zgora.pl>
To: "Luiz Augusto von Dentz" <luiz.dentz@gmail.com>
Cc: "linux-bluetooth" <linux-bluetooth@vger.kernel.org>
Message-ID: <18d356110eb.1120f313b408792.7694888926917426249@lab.zgora.pl>
In-Reply-To: <CABBYNZKW8odyLb_BQcXXpjAKjPzHRhXVCbxvcg54VkoeULeHnw@mail.gmail.com>
References: <20240121100328.1200839-1-mk@lab.zgora.pl> <CABBYNZKW8odyLb_BQcXXpjAKjPzHRhXVCbxvcg54VkoeULeHnw@mail.gmail.com>
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,

 > Hi Mariusz,=20
 > =20
 > On Sun, Jan 21, 2024 at 5:04=E2=80=AFAM Mariusz Koz=C5=82owski mk@lab.zg=
ora.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_struc=
t *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 ne=
eded.=20
=20
For version 3 caps (64 bit masks) a single struct __user_cap_data_struct is=
 not
big enough and capget() writes past the end of cap structure on the stack. =
To
accomodate version 3 cap masks the cap structure needs to be 2x bigger.

 > >         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
 > > --=20
 > > 2.34.1=20
 > >=20
 > >=20
 > =20
 > =20
 > --=20
 > Luiz Augusto von Dentz=20
 > =20
 >=20