linux-kernel - Re: [EXTERNAL] Re: [PATCH net-next v2 3/3] net: ti: icss-iep: Fix possible NULL pointer dereference for perout request

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <de68dc95-25e3-4bed-a2ab-4736a83867a2@ti.com>
Date: Fri, 28 Mar 2025 15:53:02 +0530
From: "Malladi, Meghana" <m-malladi@...com>
To: Dan Carpenter <dan.carpenter@...aro.org>
CC: Jakub Kicinski <kuba@...nel.org>, <pabeni@...hat.com>,
        <edumazet@...gle.com>, <davem@...emloft.net>, <andrew+netdev@...n.ch>,
        <bpf@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <netdev@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>,
        <kory.maincent@...tlin.com>, <javier.carrasco.cruz@...il.com>,
        <diogo.ivo@...mens.com>, <jacob.e.keller@...el.com>,
        <horms@...nel.org>, <john.fastabend@...il.com>, <hawk@...nel.org>,
        <daniel@...earbox.net>, <ast@...nel.org>, <srk@...com>,
        Vignesh Raghavendra <vigneshr@...com>,
        Roger
 Quadros <rogerq@...nel.org>, <danishanwar@...com>
Subject: Re: [EXTERNAL] Re: [PATCH net-next v2 3/3] net: ti: icss-iep: Fix
 possible NULL pointer dereference for perout request



On 3/28/2025 1:32 PM, Dan Carpenter wrote:
> On Fri, Mar 28, 2025 at 11: 46: 49AM +0530, Malladi, Meghana wrote: > > 
>  > On 3/25/2025 11: 18 PM, Jakub Kicinski wrote: > > On Fri, 21 Mar 2025 
> 13: 43: 13 +0530 Meghana Malladi wrote: > > > Whenever there is a perout 
> request
> ZjQcmQRYFpfptBannerStart
> This message was sent from outside of Texas Instruments.
> Do not click links or open attachments unless you recognize the source 
> of this email and know the content is safe.
> Report Suspicious
> <https://us-phishalarm-ewt.proofpoint.com/EWT/v1/G3vK! 
> uldqfVdF3CcxCkXE1pKJH1UXrlua- 
> GvI0DSoHvw1l5Yyu6xCbgDWX8PlhMSuV5lCv48fT7ChvXbQ105c6PHChn2lWCWZMsMcoHjvVwM$>
> ZjQcmQRYFpfptBannerEnd
> 
> On Fri, Mar 28, 2025 at 11:46:49AM +0530, Malladi, Meghana wrote:
>> 
>> 
>> On 3/25/2025 11:18 PM, Jakub Kicinski wrote:
>> > On Fri, 21 Mar 2025 13:43:13 +0530 Meghana Malladi wrote:
>> > > Whenever there is a perout request from the user application,
>> > > kernel receives req structure containing the configuration info
>> > > for that req.
>> > 
>> > This doesn't really explain the condition under which the bug triggers.
>> > Presumably when user request comes in req is never NULL?
>> > 
>> 
>> You are right, I have looked into what would trigger this bug but seems like
>> user request can never be NULL, but the contents inside the req can be
>> invalid, but that is already being handled by the kernel. So this bug fix
>> makes no sense and I will be dropping this patch for v3. Thanks.
>> 
> 
> I don't remember bug reports for more than a few hours so I had to dig
> this up on lore:
> 
> https://urldefense.com/v3/__https://lore.kernel.org/ 
> all/7b1c7c36-363a-4085-b26c-4f210bee1df6@...nley.mountain/__;!!G3vK! 
> XPLUWNuB49W9FXpnac95FM8thR9_zMOBwt7JgYy8Yaf72LIm4Xt-3Yc8h1sEti5uVdguSWQhcfsnC1_ymQIMTg$ <https://urldefense.com/v3/__https://lore.kernel.org/all/7b1c7c36-363a-4085-b26c-4f210bee1df6@stanley.mountain/__;!!G3vK!XPLUWNuB49W9FXpnac95FM8thR9_zMOBwt7JgYy8Yaf72LIm4Xt-3Yc8h1sEti5uVdguSWQhcfsnC1_ymQIMTg$>
> 
> This is definitely still a real bug on today's linux-next but yes, the
> fix is bad.
> 
> drivers/net/ethernet/ti/icssg/icss_iep.c
>     814  int icss_iep_exit(struct icss_iep *iep)
>     815  {
>     816          if (iep->ptp_clock) {
>     817                  ptp_clock_unregister(iep->ptp_clock);
>     818                  iep->ptp_clock = NULL;
>     819          }
>     820          icss_iep_disable(iep);
>     821
>     822          if (iep->pps_enabled)
>     823                  icss_iep_pps_enable(iep, false);
>     824          else if (iep->perout_enabled)
>     825                  icss_iep_perout_enable(iep, NULL, false);
>                                                      ^^^^
> A better fix probably to delete this function call instead of
> turning it into a no-op.

Yes agreed, current bug fix is bad. Will have a fix similar to this and 
post it soon. Thanks.

> 
>     826
>     827          return 0;
>     828  }
> 
> regards,
> dan carpenter
>