Re: apparent bug about check_free_strict
Toomas Soome <[email protected]> Tue, 25 Nov 2025 16:28:03 +0200
| Newsgroups | org.kernel.vger.smatch |
|---|---|
| Message-ID | <[email protected]> |
And another interesting case:
smatch is complaining about about =E2=80=98pptr=E2=80=99 but we do free =
=E2=80=98ptr=E2=80=99.=20
=
/code/illumos-gate/usr/src/tools/proto/root_i386-nd/opt/onbld/bin/i386/sma=
tch: adm_kef_util.c:1243 filter_mechlist() error: dereferencing freed =
memory 'pptr' (line 1242)
1225 filter_mechlist(mechlist_t **pmechlist, const char *mech)
1226 {
1227 int cnt =3D 0;
1228 mechlist_t *ptr, *pptr;
1229 boolean_t mech_present =3D B_FALSE;
1230 =20
1231 ptr =3D pptr =3D *pmechlist;
1232 =20
1233 while (ptr !=3D NULL) {
1234 if (strncmp(ptr->name, mech, sizeof (mech_name_t)) =
=3D=3D 0) {
1235 mech_present =3D B_TRUE;
1236 if (ptr =3D=3D *pmechlist) {
1237 pptr =3D *pmechlist =3D ptr->next;
1238 free(ptr);
1239 ptr =3D pptr;
1240 } else {
1241 pptr->next =3D ptr->next;
1242 free(ptr);
1243 ptr =3D pptr->next;
1244 }
1245 } else {
1246 pptr =3D ptr;
1247 ptr =3D ptr->next;
1248 cnt++;
1249 }
1250 }
1251 =20
1252 /* Only one entry is present */
1253 if (cnt =3D=3D 0)
1254 *pmechlist =3D NULL;
1255 =20
1256 return (mech_present);
1257 }
thanks,
toomas
> On 25. Nov 2025, at 15:38, Toomas Soome <[email protected]> wrote:
>=20
> Sorry, forgot to make it plain text again:D
>=20
>=20
> Ok, i did pull your latest update, and the problem is still there. I =
actually got very graphical example:
>=20
> =
/code/illumos-gate/usr/src/tools/proto/root_i386-nd/opt/onbld/bin/i386/sma=
tch: ../smb_share_doorclnt.c:274 smb_share_count() warn: passing freed =
memory 'dec_ctx' (line 272)
> =
/code/illumos-gate/usr/src/tools/proto/root_i386-nd/opt/onbld/bin/i386/sma=
tch: ../smb_share_doorclnt.c:279 smb_share_count() warn: passing freed =
memory 'dec_ctx' (line 272)
>=20
> now, the code in question is:
>=20
> 246 smb_share_count(void)
> 247 {
> 248 door_arg_t *arg;
> 249 smb_dr_ctx_t *dec_ctx;
> 250 smb_dr_ctx_t *enc_ctx;
> 251 uint32_t num_shares;
> 252 int rc;
> 253=20
> 254 if ((arg =3D smb_share_door_clnt_enter()) =3D=3D NULL)
> 255 return (-1);
> 256=20
> 257 enc_ctx =3D smb_dr_encode_start(arg->data_ptr, =
SMB_SHARE_DSIZE);
> 258 smb_dr_put_uint32(enc_ctx, SMB_SHROP_NUM_SHARES);
> 259=20
> 260 rc =3D smb_dr_encode_finish(enc_ctx, (unsigned int =
*)&arg->data_size);
> 261 if (rc !=3D 0) {
> 262 smb_share_door_clnt_exit(arg, B_FALSE, "encode");
> 263 return (-1);
> 264 }
> 265=20
> 266 if (smb_share_door_call(smb_share_dfd, arg) < 0) {
> 267 smb_share_door_clnt_exit(arg, B_TRUE, "door =
call");
> 268 return (-1);
> 269 }
> 270=20
> 271 dec_ctx =3D smb_dr_decode_start(arg->data_ptr, =
arg->data_size);
> 272 if (dec_ctx =3D=3D NULL || smb_share_dchk(dec_ctx) !=3D 0) =
{
> 273 if (dec_ctx !=3D NULL)
> 274 (void) smb_dr_decode_finish(dec_ctx);
> 275 smb_share_door_clnt_exit(arg, B_FALSE, "decode");
> 276 return (-1);
> 277 }
> 278=20
> 279 num_shares =3D smb_dr_get_uint32(dec_ctx);
> 280 if (smb_dr_decode_finish(dec_ctx) !=3D 0) {
> 281 smb_share_door_clnt_exit(arg, B_FALSE, "decode");
> 282 return (-1);
> 283 }
> 284=20
> 285 smb_share_door_clnt_exit(arg, B_FALSE, NULL);
> 286 return (num_shares);
> 287 }
>=20
> Both warning locations do have preceding =E2=80=9Cif" statements=E2=80=A6=
=20
>=20
> Other cases (and including the case mentioned below), the warning is =
referring to some function which does not free the pointer, so it has to =
be about some sort of context processing but so far I havent been able =
to find any logical reason to get such warning;)
>=20
> Anyhow, the example above hints there is something still a miss in =
this detection logic.
>=20
> thanks,
> toomas
>=20
>=20
>> On 24. Nov 2025, at 17:30, Toomas Soome <[email protected]> wrote:
>>=20
>>=20
>>=20
>>> On 24. Nov 2025, at 16:46, Dan Carpenter <[email protected]> =
wrote:
>>>=20
>>> Hi Toomas,
>>>=20
>>> I've recently re-written the check for use after frees. Could you
>>> retest? I have another fix for use after free which I'm going to
>>> send tomorrow hopefully...
>>>=20
>>> regards,
>>> dan carpenter
>>>=20
>>>=20
>>=20
>> I do have the latest HEAD and yes, it is still there. It=E2=80=99s =
not a single isolated case, on whole illumos kernel tree its 10 cases.
>>=20
>> It *may* be related, but I have stepped on on some curious SIGSEGV =
case too:
>>=20
>> tsoome@balrog:/code/illumos-gate/usr/src/cmd/diskinfo$ mdb core=20
>> Loading modules: [ libumem.so.1 libc.so.1 ld.so.1 ]
>>> ::stack -t
>> struct expression * cast_expression+0xf((struct expression *)NULL, =
(struct symbol *)fffff7ffe9334610)
>> void fake_return_assignment+0x74((struct db_callback_info =
*)fffff7ffffdf0520, (int)405, (int)ffffffff, (char *)1a31938, (char =
*)1a31e38)
>> int db_assign_return_states_callback+0x2ef((void *)fffff7ffffdf0520, =
(int)6, (char **)1a324e8, (char **)1a324b8)
>> int libsqlite3.so.3.50.4`sqlite3_exec+0x559()
>> void sql_exec+0x1b6((struct sqlite3 *)1a0ab98, (int (*)())4e543f, =
(void *)fffff7ffffdf0520, (const char *)fffff7ffffdefd70)
>> void sql_select_return_states+0x396((const char *)5a2908, (struct =
expression *)fffff7ffec1d6960, (int (*)())4e543f, (void =
*)fffff7ffffdf0520)
>> int db_return_states_assign+0x89((struct expression =
*)fffff7ffebff5e70)
>> void match_assign_call+0x70((struct expression *)fffff7ffebff5e70)
>> void __pass_to_client+0x57((void *)fffff7ffebff5e70, (enum =
hook_type)CALL_ASSIGNMENT_HOOK)
>> void parse_assignment+0x2a0((struct expression *)fffff7ffebff5e70, =
(_Bool)0)
>> void __split_expr+0x317((struct expression *)fffff7ffebff5e70)
>> void __split_expr+0x644((struct expression *)fffff7ffec1d6780)
>> void parse_assignment+0x134((struct expression *)fffff7ffebff5ce0, =
(_Bool)0)
>> void __split_expr+0x317((struct expression *)fffff7ffebff5ce0)
>> void __split_expr+0x578((struct expression *)fffff7ffec1d6780)
>> void __split_stmt+0x3e2((struct statement *)fffff7ffea4849f8)
>> void __split_stmt+0x4c8((struct statement *)fffff7ffea4849a0)
>> void __split_stmt+0x56b((struct statement *)fffff7ffea484948)
>> void __split_stmt+0x4c8((struct statement *)fffff7ffea4848f0)
>> void __split_stmt+0x56b((struct statement *)fffff7ffea484898)
>> void __split_stmt+0x442((struct statement *)fffff7ffea4828a0)
>> void handle_pre_loop+0x17f((struct statement *)fffff7ffea482848)
>> void __split_stmt+0x886((struct statement *)fffff7ffea482848)
>> void __split_stmt+0x442((struct statement *)fffff7ffea482060)
>> void split_function+0x113((struct symbol *)fffff7ffeb3c0d30)
>> void smatch+0x3e3((struct string_list *)fffff7ffef2c1590)
>> int main+0x18f((int)33, (char **)fffff7ffffdf1ea8)
>> _start_crt+0x87()
>> _start+0x18()
>>>=20
>>=20
>> there we also do get some data from sql but for some reason we end up =
passing NULL pointer to cast_expression where it is not expected. I=E2=80=99=
m trying to figure out how to spot the location from source code the =
smatch is processing there, some hints about it would be much welcome;)
>>=20
>> rgds,
>> toomas
>=20