Sanitizer test #321

cgzones · 2021-10-12T14:18:26Z

No description provided.

cgzones · 2021-10-12T14:26:47Z

evverx · 2021-10-12T15:06:33Z

@cgzones Looks like UBSan no longer complains about those null pointers. Thanks!

Regarding memory leaks, unfortunately there're a lot of them in the tests themselves so I think until they're fixed they probably shouldn't be reported so as not to clutter the output of the tests.

evverx · 2021-10-12T15:37:39Z

I think until they're fixed they probably shouldn't be reported so as not to clutter the output of the tests.

Having said that, looking at the "raw" logs of the action I think it's more or less manageable so I'm going to have to take that back.

evverx · 2021-10-14T14:54:57Z

@cgzones I noticed that you sent the patches where all the issues found by ASan/UBsan were fixed to the mailing list. Are you planning to send the other commits later or should I just cherry-pick the two commits you added related to GHActions on top of my PR and send it myself?

cgzones · 2021-10-15T12:54:55Z

Are you planning to send the other commits later or should I just cherry-pick the two commits you added related to GHActions on top of my PR and send it myself?

I'd appreciate if you could send them yourself.
(Probably there is no need to cherry-pick my patches: ci: enable leak sanitizer is just a partial revert of ci: ignore memory leaks for now and ci: build all subdirectories in sanitizer job except python and sandbox is void, since all of those sub directories have no tests.)

evverx · 2021-10-15T14:39:32Z

I'll squash the commits into one and send it to the mailing list once https://lore.kernel.org/selinux/[email protected]/T/#t is merged to keep the GHActions workflow green. Thanks!

fishilico · 2021-10-16T19:53:33Z

libsemanage/tests/test_ibendport.c

@@ -362,6 +373,8 @@ void test_ibendport_modify_del_query_local(void)
 &ibendport_local) < 0);

 /* cleanup */
+ semanage_ibendport_key_free(key);
+ semanage_ibendport_free(ibendport_local);


@cgzones This semanage_ibendport_free(ibendport_local); should probably be moved after CU_ASSERT_PTR_NOT_NULL_FATAL(ibendport_local); (a few lines up), as the variable is then used again in the test which checks that semanage_ibendport_query_local fails,

CU_ASSERT(semanage_ibendport_query_local(sh, key, &ibendport_local) < 0);

true, I'll reorder this and the other affected free calls

fishilico · 2021-10-16T19:59:49Z

libsemanage/tests/test_user.c

@@ -577,6 +590,8 @@ void test_user_modify_del_query_local(void)
 CU_ASSERT(semanage_user_query_local(sh, key, &user_local) < 0);

 /* cleanup */
+ semanage_user_key_free(key);
+ semanage_user_free(user_local);


Same comment as 322243e#r730306247 here: semanage_user_free(user_local); should be moved up a few lines, before CU_ASSERT(semanage_user_query_local(sh, key, &user_local) < 0);

fishilico · 2021-10-16T20:02:15Z

libsemanage/tests/test_iface.c

@@ -546,6 +567,8 @@ void test_iface_modify_del_query_local(void)
 CU_ASSERT(semanage_iface_query_local(sh, key, &iface_local) < 0);

 /* cleanup */
+ semanage_iface_key_free(key);
+ semanage_iface_free(iface_local);


Same comment as 322243e#r730306247 here: semanage_iface_free(iface_local); should be moved up a few lines, before CU_ASSERT(semanage_iface_query_local(sh, key, &iface_local) < 0);

cgzones · 2021-11-16T10:08:27Z

@evverx fyi, the sanitizer patches have been applied via b98d3c4, ea53901, fe01a91 and f95dbf2

evverx · 2021-11-16T10:15:32Z

@cgzones thanks! I'll try to rebase #320 on top of the master branch to make sure it works, squash the commits into one and send it to the mailing list.

It was tested in SELinuxProject#321 and SELinuxProject#320. In the process it discovered a few issues all of which were fixed in SELinuxProject@b98d3c4 SELinuxProject@ea53901 SELinuxProject@fe01a91 SELinuxProject@f95dbf2 Now that all the issues are gone it should be safe to turn it on to make it easier to automatically catch bugs like that almost as soon as they end up in the repository. Signed-off-by: Evgeny Vereshchagin <[email protected]>

It was tested in #321 and #320. In the process it discovered a few issues all of which were fixed in b98d3c4 ea53901 fe01a91 f95dbf2 Now that all the issues are gone it should be safe to turn it on to make it easier to automatically catch bugs like that almost as soon as they end up in the repository. Signed-off-by: Evgeny Vereshchagin <[email protected]>

It was tested in SELinuxProject/selinux#321 and SELinuxProject/selinux#320. In the process it discovered a few issues all of which were fixed in SELinuxProject/selinux@b98d3c4 SELinuxProject/selinux@ea53901 SELinuxProject/selinux@fe01a91 SELinuxProject/selinux@f95dbf2 Now that all the issues are gone it should be safe to turn it on to make it easier to automatically catch bugs like that almost as soon as they end up in the repository. Signed-off-by: Evgeny VereshchaGin <[email protected]>

Signed-off-by: Christian Göttsche <[email protected]>

cgzones force-pushed the sanitizer_test branch from b5471f7 to 5261491 Compare October 12, 2021 18:41

evverx mentioned this pull request Oct 12, 2021

ci: an attempt to run the tests under ASan/UBsan on GHActions #320

Closed

cgzones force-pushed the sanitizer_test branch 4 times, most recently from 844fb24 to 829f618 Compare October 13, 2021 12:45

fishilico reviewed Oct 16, 2021

View reviewed changes

cgzones force-pushed the sanitizer_test branch from 829f618 to 54c3dde Compare January 15, 2022 17:23

cgzones force-pushed the sanitizer_test branch from 54c3dde to 5bb2b8d Compare March 5, 2022 19:34

cgzones force-pushed the sanitizer_test branch from 5bb2b8d to 7c1a710 Compare March 31, 2022 13:16

cgzones force-pushed the sanitizer_test branch from 7c1a710 to 1cdac3b Compare April 20, 2022 19:57

cgzones force-pushed the sanitizer_test branch from 1cdac3b to d21dc46 Compare June 7, 2022 17:02

cgzones force-pushed the sanitizer_test branch from d21dc46 to 072b2e3 Compare June 15, 2022 18:50

cgzones force-pushed the sanitizer_test branch 2 times, most recently from f557dab to a89dad1 Compare July 12, 2022 16:17

cgzones force-pushed the sanitizer_test branch from a89dad1 to 3fc0f42 Compare August 30, 2022 16:19

cgzones force-pushed the sanitizer_test branch from 3fc0f42 to 7065496 Compare November 23, 2022 19:05

cgzones force-pushed the sanitizer_test branch from 7065496 to 67a55c8 Compare April 20, 2023 15:22

cgzones force-pushed the sanitizer_test branch from 67a55c8 to e942844 Compare May 12, 2023 09:31

cgzones force-pushed the sanitizer_test branch from e942844 to 8300e2f Compare July 6, 2023 14:02

cgzones force-pushed the sanitizer_test branch from 8300e2f to 343d13a Compare July 14, 2023 18:36

cgzones added 8 commits November 1, 2023 14:51

libsemanage: free resources on failed connect attempt

792af02

Signed-off-by: Christian Göttsche <[email protected]>

ci: build all subdirectories in sanitizer job except python and sandbox

260645f

Signed-off-by: Christian Göttsche <[email protected]>

ci: use -fsanitize-address-use-after-scope in sanitizer job

b66147f

Signed-off-by: Christian Göttsche <[email protected]>

Drop -z,defs linker flags

b53056b

Signed-off-by: Christian Göttsche <[email protected]>

ignore ebitmap math

46dc50a

Signed-off-by: Christian Göttsche <[email protected]>

libsemanage/tests: mark unused function parameters

eed5508

Signed-off-by: Christian Göttsche <[email protected]>

libsemanage: silence underflow

2aee578

Signed-off-by: Christian Göttsche <[email protected]>

libsemanage/tests: silence function UBSAN

dc85199

Signed-off-by: Christian Göttsche <[email protected]>

cgzones force-pushed the sanitizer_test branch from 343d13a to dc85199 Compare November 1, 2023 14:39

cgzones changed the base branch from master to main November 1, 2023 17:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sanitizer test #321

Sanitizer test #321

cgzones commented Oct 12, 2021

cgzones commented Oct 12, 2021

evverx commented Oct 12, 2021

evverx commented Oct 12, 2021

evverx commented Oct 14, 2021

cgzones commented Oct 15, 2021

evverx commented Oct 15, 2021

fishilico Oct 16, 2021 •

edited

cgzones Oct 19, 2021

fishilico Oct 16, 2021

fishilico Oct 16, 2021

cgzones commented Nov 16, 2021

evverx commented Nov 16, 2021 •

edited

Sanitizer test #321

Are you sure you want to change the base?

Sanitizer test #321

Conversation

cgzones commented Oct 12, 2021

cgzones commented Oct 12, 2021

evverx commented Oct 12, 2021

evverx commented Oct 12, 2021

evverx commented Oct 14, 2021

cgzones commented Oct 15, 2021

evverx commented Oct 15, 2021

fishilico Oct 16, 2021 • edited

Choose a reason for hiding this comment

cgzones Oct 19, 2021

Choose a reason for hiding this comment

fishilico Oct 16, 2021

Choose a reason for hiding this comment

fishilico Oct 16, 2021

Choose a reason for hiding this comment

cgzones commented Nov 16, 2021

evverx commented Nov 16, 2021 • edited

fishilico Oct 16, 2021 •

edited

evverx commented Nov 16, 2021 •

edited