Bug 15224 - pam_winbind uses time_t and pointers assuming they are of the same size
Summary: pam_winbind uses time_t and pointers assuming they are of the same size
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: Winbind (show other bugs)
Version: 4.17.1
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Jule Anger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-11-03 20:07 UTC by Michael Tokarev
Modified: 2022-12-15 16:35 UTC (History)
2 users (show)

See Also:

Attachments
git-am fix for master. (2.63 KB, patch)
2022-11-09 00:18 UTC, Jeremy Allison 		no flags Details
git-am fix for 4.17.next, 4.16.next. (4.02 KB, patch)
2022-11-16 20:47 UTC, Jeremy Allison 		npower: review+
Details
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Tokarev 2022-11-03 20:07:23 UTC 
In nsswitch/pam_winbind.c, there are 2 places like this:

               time_t pwdlastset_prelim [ = some value];

# pam_winbind.c 3271
                pam_set_data(pamh, PAM_WINBIND_PWD_LAST_SET,
                             (void *)pwdlastset_prelim, NULL);


                time_t pwdlastset_update = 0;

# pam_winbind.c 3346
                pam_get_data(pamh, PAM_WINBIND_PWD_LAST_SET,
                             (const void **) &pwdlastset_update);

It is not guaranteed that time_t is exactly of the same size as a pointer.  There are already architecture(s) today which breaks this assumption, for example x32, where time_t is 8 bytes but pointer is 4 bytes, so we're storing only the half of the value there.  This breaks with musl where time_t is always 64bit no matter which size a pointer is.

Here, either a pointer to an allocated time_t should be stored (maybe conditionally, I dunno), or some other time representation should be used.  The latter - the pam_get_data case - even looks dangerous, - I'd use a void* variable, pass its address to pam_get_data, and use direct cast from pointer to time_t, instead of passing an address to an object of potentially different size.
Comment 1 Jeremy Allison 2022-11-09 00:18:02 UTC 
Created attachment 17639 [details]
git-am fix for master.

Michael, does this fix work for you ? If you can confirm I'll add it as an MR and try and get it reviewed and merged.
Comment 2 Jeremy Allison 2022-11-11 19:31:49 UTC 
From: Michael Tokarev <mjt@tls.msk.ru>
To: Jeremy Allison <jra@samba.org>
Subject: Re: Can you look at the patch on bug
        https://bugzilla.samba.org/show_bug.cgi?id=15224

Hello again!  As promised, I'm following up.  It looks good so far.
I also tested it on a real system, - things are looking promising.
Comment 3 Samba QA Contact 2022-11-16 15:10:03 UTC 
This bug was referenced in samba master:

7cb50405515298b75dcc512633fb3877045aabc6
Comment 4 Samba QA Contact 2022-11-16 19:30:04 UTC 
This bug was referenced in samba master:

f6284877ce07fc5ddf4f4e2d824013b645d6e12c
Comment 5 Jeremy Allison 2022-11-16 20:47:20 UTC 
Created attachment 17661 [details]
git-am fix for 4.17.next, 4.16.next.

Cherry-picked from master.
Comment 6 Noel Power 2022-11-19 08:26:40 UTC 
Comment on attachment 17661 [details]
git-am fix for 4.17.next, 4.16.next.

lgtm
Comment 7 Noel Power 2022-11-19 08:27:24 UTC 
reassign to Jule for inclusion in 4.16/17
Comment 8 Jule Anger 2022-11-23 07:17:08 UTC 
Pushed to autobuild-v4-{17,16}-test.
Comment 9 Samba QA Contact 2022-11-23 13:53:25 UTC 
This bug was referenced in samba v4-16-test:

399522d048e5f7effec31c67588fac722a970f8d
33f74aea5d5d8096dfd71089a74a123161957197
Comment 10 Samba QA Contact 2022-11-23 13:57:19 UTC 
This bug was referenced in samba v4-17-test:

d7e34c8b1578e4cc2c30a5a035c5eddfd0e1e1f1
50fd29d8b88226819208a8215867d81c1edc78a9
Comment 11 Jule Anger 2022-11-23 14:51:20 UTC 
Closing out bug report.

Thanks!
Comment 12 Samba QA Contact 2022-12-15 16:34:37 UTC 
This bug was referenced in samba v4-16-stable (Release samba-4.16.8):

399522d048e5f7effec31c67588fac722a970f8d
33f74aea5d5d8096dfd71089a74a123161957197
Comment 13 Samba QA Contact 2022-12-15 16:35:56 UTC 
This bug was referenced in samba v4-17-stable (Release samba-4.17.4):

d7e34c8b1578e4cc2c30a5a035c5eddfd0e1e1f1
50fd29d8b88226819208a8215867d81c1edc78a9