[PATCH] qmi: avoid free of non heap object

[PATCH] qmi: avoid free of non heap object

[Sean Nyekjaer]

qmux_create_client_timeout() free's a non heap object, avoid calling
__request_free() and inline free_request() instead.

In function 'l_free',
    inlined from '__request_free' at drivers/qmimodem/qmi.c:293:3,
    inlined from 'qmux_create_client_timeout' at drivers/qmimodem/qmi.c:1427:2:
ell/util.c:124:9: warning: 'free' called on pointer 'user_data_6(D)' with nonzero offset 28 [-Wfree-nonheap-object]
  124 |         free(ptr);
      |         ^
---
 drivers/qmimodem/qmi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/qmimodem/qmi.c b/drivers/qmimodem/qmi.c
index 5d2553c5..4468761a 100644
--- a/drivers/qmimodem/qmi.c
+++ b/drivers/qmimodem/qmi.c
@@ -1424,7 +1424,9 @@ static void qmux_create_client_timeout(struct l_timeout *timeout,
 
 	/* remove request from queues */
 	find_control_request(qmux, req->super.tid);
-	__request_free(&req->super);
+
+	if (req->super.free_request)
+		req->super.free_request(&req->super);
 }
 
 static void qmux_create_client_callback(struct qmi_request *r,
-- 
2.49.0

Re: [PATCH] qmi: avoid free of non heap object

[Denis Kenzior]

Hi Sean,

On 6/26/25 3:16 AM, Sean Nyekjaer wrote:
> qmux_create_client_timeout() free's a non heap object, avoid calling
> __request_free() and inline free_request() instead.
> 
> In function 'l_free',
>      inlined from '__request_free' at drivers/qmimodem/qmi.c:293:3,
>      inlined from 'qmux_create_client_timeout' at drivers/qmimodem/qmi.c:1427:2:
> ell/util.c:124:9: warning: 'free' called on pointer 'user_data_6(D)' with nonzero offset 28 [-Wfree-nonheap-object]
>    124 |         free(ptr);
>        |         ^

Hmm, what tool is producing this output?

> ---
>   drivers/qmimodem/qmi.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/qmimodem/qmi.c b/drivers/qmimodem/qmi.c
> index 5d2553c5..4468761a 100644
> --- a/drivers/qmimodem/qmi.c
> +++ b/drivers/qmimodem/qmi.c
> @@ -1424,7 +1424,9 @@ static void qmux_create_client_timeout(struct l_timeout *timeout,
>   
>   	/* remove request from queues */
>   	find_control_request(qmux, req->super.tid);
> -	__request_free(&req->super);
> +
> +	if (req->super.free_request)
> +		req->super.free_request(&req->super);

This must be some sort of false positive.  In qmi_qmux_device_create_client() we 
always set .free_request, so __request_free will never take the l_free() branch.

The question is whether a compiler is producing this output or some static 
analysis tool?

>   }
>   
>   static void qmux_create_client_callback(struct qmi_request *r,

Regards,
-Denis

Re: [PATCH] qmi: avoid free of non heap object

[Sean Nyekjaer]

Hi Denis,

On Thu, Jun 26, 2025 at 10:11:34AM +0100, Denis Kenzior wrote:
> Hi Sean,
> 
> On 6/26/25 3:16 AM, Sean Nyekjaer wrote:
> > qmux_create_client_timeout() free's a non heap object, avoid calling
> > __request_free() and inline free_request() instead.
> >
> > In function 'l_free',
> >      inlined from '__request_free' at drivers/qmimodem/qmi.c:293:3,
> >      inlined from 'qmux_create_client_timeout' at drivers/qmimodem/qmi.c:1427:2:
> > ell/util.c:124:9: warning: 'free' called on pointer 'user_data_6(D)' with nonzero offset 28 [-Wfree-nonheap-object]
> >    124 |         free(ptr);
> >        |         ^
> 
> Hmm, what tool is producing this output?

Good old gcc:
libtool: link: arm-oe-linux-gnueabi-gcc -mthumb -mfpu=neon -mfloat-abi=hard -mcpu=cortex-a7 -D_TIME_BITS=64 -D_FILE_OFFSET_BITS=64 --sysroot=/usr/local/cc-sdk/sysroots/cortexa7t2hf-neon-oe-linux-gnueabi -I/usr/local/cc-sdk/sysroots/cortexa7t2hf-neon-oe-linux-gnueabi/usr/include/dbus-1.0 -I/usr/local/cc-sdk/sysroots/cortexa7t2hf-neon-oe-linux-gnueabi/usr/lib/dbus-1.0/include -I/usr/local/cc-sdk/sysroots/cortexa7t2hf-neon-oe-linux-gnueabi/usr/include/glib-2.0 -I/usr/local/cc-sdk/sysroots/cortexa7t2hf-neon-oe-linux-gnueabi/usr/lib/glib-2.0/include -DOFONO_PLUGIN_BUILTIN -DPLUGINDIR=\"/usr/local/lib/ofono/plugins\" -DUNITDIR=\"./unit/\" -O2 -pipe -g -feliminate-unused-debug-types -flto -ffat-lto-objects -fuse-linker-plugin -O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wl,--export-dynamic -Wl,--version-script=./src/ofono.ver -Wl,-O1 -Wl,--hash-style=gnu -Wl,--as-needed -o src/ofonod plugins/udevng.o src/rmnet.o drivers/qmimodem/qmi.o drivers/qmimodem/nas.o drivers/qmimodem/wda.o drivers/qmimodem/wds.o drivers/qmimodem/devinfo.o drivers/qmimodem/voicecall.o drivers/qmimodem/network-registration.o drivers/qmimodem/sim-legacy.o drivers/qmimodem/sim.o drivers/qmimodem/sms.o drivers/qmimodem/ussd.o drivers/qmimodem/gprs.o drivers/qmimodem/gprs-context.o drivers/qmimodem/lte.o drivers/qmimodem/radio-settings.o drivers/qmimodem/location-reporting.o drivers/qmimodem/netmon.o drivers/qmimodem/call-settings.o drivers/qmimodem/call-barring.o drivers/qmimodem/call-forwarding.o plugins/gobi.o plugins/qrtrqmi.o drivers/atmodem/call-settings.o drivers/atmodem/sms.o drivers/atmodem/cbs.o drivers/atmodem/call-forwarding.o drivers/atmodem/call-meter.o drivers/atmodem/network-registration.o drivers/atmodem/sim.o drivers/atmodem/stk.o drivers/atmodem/ussd.o drivers/atmodem/voicecall.o drivers/atmodem/call-barring.o drivers/atmodem/phonebook.o drivers/atmodem/devinfo.o drivers/atmodem/call-volume.o drivers/atmodem/atutil.o drivers/atmodem/gprs.o drivers/atmodem/gprs-context.o drivers/atmodem/gnss.o drivers/atmodem/lte.o drivers/nwmodem/radio-settings.o drivers/swmodem/gprs-context.o drivers/ztemodem/radio-settings.o drivers/iceramodem/gprs-context.o drivers/iceramodem/radio-settings.o drivers/huaweimodem/ussd.o drivers/huaweimodem/voicecall.o drivers/huaweimodem/audio-settings.o drivers/huaweimodem/gprs-context.o drivers/huaweimodem/radio-settings.o drivers/calypsomodem/voicecall.o drivers/calypsomodem/stk.o drivers/mbmmodem/gprs-context.o drivers/mbmmodem/stk.o drivers/mbmmodem/location-reporting.o drivers/telitmodem/location-reporting.o drivers/telitmodem/gprs-context-ncm.o drivers/hsomodem/gprs-context.o drivers/hsomodem/radio-settings.o drivers/ifxmodem/voicecall.o drivers/ifxmodem/audio-settings.o drivers/ifxmodem/radio-settings.o drivers/ifxmodem/gprs-context.o drivers/ifxmodem/stk.o drivers/ifxmodem/ctm.o drivers/stemodem/stemodem.o drivers/stemodem/voicecall.o drivers/stemodem/radio-settings.o drivers/stemodem/caif_rtnl.o drivers/stemodem/gprs-context.o drivers/dunmodem/network-registration.o drivers/dunmodem/gprs.o drivers/hfpmodem/slc.o drivers/hfpmodem/voicecall.o drivers/hfpmodem/network-registration.o drivers/hfpmodem/call-volume.o drivers/hfpmodem/devinfo.o drivers/hfpmodem/handsfree.o drivers/hfpmodem/siri.o drivers/speedupmodem/ussd.o drivers/ubloxmodem/ubloxmodem.o drivers/ubloxmodem/gprs-context.o drivers/ubloxmodem/network-registration.o drivers/ubloxmodem/netmon.o drivers/ubloxmodem/lte.o drivers/gemaltomodem/location-reporting.o drivers/gemaltomodem/voicecall.o drivers/gemaltomodem/gprs-context.o drivers/gemaltomodem/radio-settings.o drivers/gemaltomodem/netmon.o drivers/xmm7modem/radio-settings.o drivers/xmm7modem/ims.o drivers/xmm7modem/netmon.o drivers/quectelmodem/radio-settings.o drivers/simcommodem/radio-settings.o plugins/g1.o plugins/wavecom.o plugins/calypso.o plugins/mbm.o plugins/hso.o plugins/zte.o plugins/huawei.o plugins/sierra.o plugins/novatel.o plugins/palmpre.o plugins/ifx.o plugins/ste.o plugins/cinterion.o plugins/gemalto.o plugins/nokia.o plugins/linktop.o plugins/icera.o plugins/alcatel.o plugins/speedup.o plugins/samsung.o plugins/sim900.o plugins/sim7100.o plugins/telit.o plugins/quectel.o plugins/ublox.o plugins/xmm7xxx.o plugins/droid.o plugins/stemgr.o plugins/connman.o plugins/hfp_hf_bluez5.o plugins/dun_gw_bluez5.o plugins/bluez5.o plugins/hfp_ag_bluez5.o plugins/upower.o plugins/smart-messaging.o plugins/push-notification.o plugins/allowed-apns.o gatchat/gatchat.o gatchat/gatresult.o gatchat/gatsyntax.o gatchat/ringbuffer.o gatchat/gatio.o gatchat/crc-ccitt.o gatchat/gatmux.o gatchat/gsm0710.o gatchat/gattty.o gatchat/gatutil.o gatchat/gatserver.o gatchat/gatrawip.o gatchat/gathdlc.o gatchat/gatppp.o gatchat/ppp_cp.o gatchat/ppp_lcp.o gatchat/ppp_auth.o gatchat/ppp_net.o gatchat/ppp_ipcp.o gatchat/ppp_ipv6cp.o src/main.o src/log.o src/plugin.o src/modem.o src/common.o src/manager.o src/dbus.o src/util.o src/network.o src/voicecall.o src/ussd.o src/sms.o src/call-settings.o src/call-forwarding.o src/call-meter.o src/smsutil.o src/call-barring.o src/sim.o src/stk.o src/phonebook.o src/history.o src/message-waiting.o src/simutil.o src/storage.o src/cbs.o src/watch.o src/call-volume.o src/gprs.o src/radio-settings.o src/stkutil.o src/nettime.o src/stkagent.o src/simfs.o src/audio-settings.o src/smsagent.o src/ctm.o src/sim-auth.o src/message.o src/emulator.o src/location-reporting.o src/gnss.o src/gnssagent.o src/private-network.o src/handsfree.o src/handsfree-audio.o src/siri.o src/netmon.o src/lte.o src/ims.o src/netmonagent.o src/module.o src/provisiondb.o src/provision.o  gdbus/.libs/libgdbus-internal.a -ludev ell/.libs/libell-internal.a -lglib-2.0 -ldbus-1 -ldl
In function 'l_free',
    inlined from '__request_free' at drivers/qmimodem/qmi.c:293:3,
    inlined from 'qmux_create_client_timeout' at drivers/qmimodem/qmi.c:1427:2:
ell/util.c:124:9: warning: 'free' called on pointer 'user_data_6(D)' with nonzero offset 28 [-Wfree-nonheap-object]
  124 |         free(ptr);
      |         ^
> 
> > ---
> >   drivers/qmimodem/qmi.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/qmimodem/qmi.c b/drivers/qmimodem/qmi.c
> > index 5d2553c5..4468761a 100644
> > --- a/drivers/qmimodem/qmi.c
> > +++ b/drivers/qmimodem/qmi.c
> > @@ -1424,7 +1424,9 @@ static void qmux_create_client_timeout(struct l_timeout *timeout,
> >
> >   	/* remove request from queues */
> >   	find_control_request(qmux, req->super.tid);
> > -	__request_free(&req->super);
> > +
> > +	if (req->super.free_request)
> > +		req->super.free_request(&req->super);
> 
> This must be some sort of false positive.  In qmi_qmux_device_create_client() we
> always set .free_request, so __request_free will never take the l_free() branch.
> 
> The question is whether a compiler is producing this output or some static
> analysis tool?

Looks like it's because we are building with the lto enabled.

> 
> >   }
> >
> >   static void qmux_create_client_callback(struct qmi_request *r,
> 
> Regards,
> -Denis

/Sean

Re: [PATCH] qmi: avoid free of non heap object

[patchwork-bot+ofono]

Hello:

This patch was applied to ofono.git (master)
by Denis Kenzior <denkenz@gmail.com>:

On Thu, 26 Jun 2025 10:16:24 +0200 you wrote:
> qmux_create_client_timeout() free's a non heap object, avoid calling
> __request_free() and inline free_request() instead.
> 
> In function 'l_free',
>     inlined from '__request_free' at drivers/qmimodem/qmi.c:293:3,
>     inlined from 'qmux_create_client_timeout' at drivers/qmimodem/qmi.c:1427:2:
> ell/util.c:124:9: warning: 'free' called on pointer 'user_data_6(D)' with nonzero offset 28 [-Wfree-nonheap-object]
>   124 |         free(ptr);
>       |         ^
> 
> [...]

Here is the summary with links:
  - qmi: avoid free of non heap object
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=8d892785e47b

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html