Received-SPF: pass (google.com: domain of linux-wireless+bounces-4486-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240307223849.13d5b58b@barney> <20240307232927.171197-1-rand.sec96@gmail.com>
 <CAAMvbhE53upfqfB=afJK9YAZUWFTBN8ripae0W+PUUXdwj4-kw@mail.gmail.com>
In-Reply-To: <CAAMvbhE53upfqfB=afJK9YAZUWFTBN8ripae0W+PUUXdwj4-kw@mail.gmail.com>
From: Rand Deeb <rand.sec96@gmail.com>
Date: Fri, 8 Mar 2024 15:11:06 +0300
Message-ID: <CAN8dotnMCQG+cUxKR_aq6=XxqMTh7KLDCpH1sOa6RqujKfYRgg@mail.gmail.com>
Subject: Re: [PATCH v3] ssb: Fix potential NULL pointer dereference in ssb_device_uevent
To: James Dutton <james.dutton@gmail.com>
Cc: deeb.rand@confident.ru, jonas.gorski@gmail.com, khoroshilov@ispras.ru, 
	kvalo@kernel.org, linux-kernel@vger.kernel.org, 
	linux-wireless@vger.kernel.org, lvc-project@linuxtesting.org, 
	voskresenski.stanislav@confident.ru
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Mar 8, 2024 at 4:05=E2=80=AFAM James Dutton <james.dutton@gmail.com=
> wrote:
> The reason it is probably not a good design is what comes later.
> Another developer comes along and says I see a nice SUM(a, b);
> function that I would like to re-use in my new function I am adding.
> But that new developer introduces a bug whereby they have implemented
> their CHECK(a) wrongly which results in SUM(a, b) now being a security
> exploit point because of some very subtle bug in CHECK(a) that no one
> noticed initially.
> After a while, we might have ten functions that all re-use SUM(a, b)
> at which point it becomes too time consuming for someone to check all
> ten functions don't have bugs in their CHECK(a) steps.
> It is always easier for the safety checks to be done as close as
> possible to the potential exploit point (e.g. NULL pointer
> dereference) so that it catches all future cases of re-use of the
> function.
> For example, there exist today zero day exploits in the Linux wireless
> code that is due to the absence of these checks being done at the
> exploit point.
> The biggest problem with all this, is if I sutily (without wishing to
> give away that it is to fix a zero day exploit) submitted a patch to
> do an extra check in SUM(a, b) that I know prevents a zero day
> exploit, my patch would be rejected.

Hi James,

Thank you very much for your detailed and interesting interaction. In fact,
while I was writing the example, I expected such a comment :)

The example is just an explanation, do not take it literally, but it is
definitely better than the second method. Now, if the developer makes a
mistake using the function, this is possible, but this is not a convincing
reason to add redundant code everywhere. In addition mistakes could occur
in all scenarios.

I agree with you in general, but I will answer you in simple statements:

For this reason, modification permission should not be granted to anyone,
and for this reason there are reviewers and documentation. In the end,
Somehow, software engineering concepts are applied because this is one of
the most important projects and not a calculator project as homework for
the university.

Best Regards