Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-3737-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <1713325792-17181-1-git-send-email-quic_zijuhu@quicinc.com>
 <Zh9saGbOl-fXP6SD@hovoldconsulting.com> <5e4e62c8-d17f-45fd-9212-088eb026dc7b@quicinc.com>
 <Zh91zq13nZvH3-Yj@hovoldconsulting.com> <4dac0caa-629d-4b4d-ad54-f6ab15268524@quicinc.com>
 <Zh-JNzlskY6ZvsSm@hovoldconsulting.com> <4313bcc9-8fbf-477d-90f2-013681cdebb9@quicinc.com>
 <ZiFDWeW_-PpI38GQ@hovoldconsulting.com>
In-Reply-To: <ZiFDWeW_-PpI38GQ@hovoldconsulting.com>
From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date: Thu, 18 Apr 2024 13:07:42 -0400
Message-ID: <CABBYNZJNiDXCT=3dT9-N7z_ntn1xDKO4SiThJQW+wKmzAGbOuQ@mail.gmail.com>
Subject: Re: [PATCH] Bluetooth: qca: Fix nullptr dereference for non-serdev devices
To: Johan Hovold <johan@kernel.org>
Cc: quic_zijuhu <quic_zijuhu@quicinc.com>, marcel@holtmann.org, 
	linux-bluetooth@vger.kernel.org, johan+linaro@kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Johan,

On Thu, Apr 18, 2024 at 11:59=E2=80=AFAM Johan Hovold <johan@kernel.org> wr=
ote:
>
> For the third time, wrap your replies at 72 cols.
>
> I've reflown your reply below manually again, but you need to fix mail
> setup and habits so you can communicate with upstream using the mailing
> lists.
>
> On Wed, Apr 17, 2024 at 05:38:59PM +0800, quic_zijuhu wrote:
> > On 4/17/2024 4:32 PM, Johan Hovold wrote:
>
> > >>>> https://patchwork.kernel.org/project/bluetooth/list/?series=3D8441=
20
>
> > > In it's current form it's a vendor specific hack that is never going =
to
> > > make it upstream.
>
> > 1)
> > ioctl()'s designed purpose is to complete such non-standard config.
>
> That's irrelevant.
>
> > 2) present ioctl HCIUARTGETPROTO which is not exported is used to
> > specify which vendor protocol is used is it a a vendor specific hack?
>
> That's an existing interface, that's ABI and has clearly defined
> semantics, unlike what you are proposing.
>
> Those protocol values can never change once they've been added.
>
> > 3)
> > hci_ldisc driver don't touch user specified settings and pass it into
> > vendor driver directly does it has any problem?
>
> No, because the protocol values will never change, unlike the random
> data you're shuffling into the driver.
>
> > 4) for tool btattach. it does NOT get any board config info from
> > DT|ACPI compared with formal BT driver. so i introduce a new ioctl to
> > supplement such info when possible to make btattach work.
>
> I understand why you want this. I still think it's the wrong approach
> and in any case the interface in it's current form is not acceptable.
>
> > > For a start, you don't even make sure that the types becomes part of =
the
> > > ABI, which means that passing, say, type 5 can mean different things
> > > depending on the kernel version.
>
> > it is specified by user and ONLY be parsed by vendor device driver.
> > it is user's responsibility to specify the right value.
> > so i also don't check and care about its value and it don't need to
> > change any code for further added any new soc_types.
>
> That's not how Linux works, sorry. We never break user space so your
> type data would have to be well-defined and can never change (you can
> only add new types).
>
> > moreover, tool attach is mainly used before the final product phase.
> > namely, its is mainly used by developer and customer's evaluation.
>
> Also irrelevant. You still don't get to add random new ioctl() that
> violates the Linux ABI contract.
>
> > > Can't you just retrieve the device type from the device itself? If it=
's
> > > already powered, you should not need to know this beforehand
>
> > 1) it is the simplest and lowest risk fix
>
> No, it's a quick and dirty hack.
>
> > 2) different soc_types have different responses when read its IDS as
> > shown by qca_read_soc_version().
>
> I'm sure that can be managed. You claim that these device have a common
> protocol (qca) so it should be possible to probe for such differences.
>
> > 3) the way you mentioned will involve many changes and it also means
> > high risks for many current soc types.
>
> There's no risk as hardly anyone uses the line discipline interface
> anymore and it can currently only be used for the old ROME devices.
> Just make sure ROME still works after your change.
>
> Probing the device type should result in a better user experience, which
> I'm sure your customers will appreciate.

+1, thanks a lot for putting in the effort to explain in such detail.

--=20
Luiz Augusto von Dentz