[4/5] login: User 64-bit time on struct lastlog

Message ID 20200729205117.2925113-4-adhemerval.zanella@linaro.org
State Superseded
Headers
Series [1/5] login: Consolidate utmp and utmpx headers |

Commit Message

Adhemerval Zanella July 29, 2020, 8:51 p.m. UTC
  It is not used directly on any symbol, so there is no need to add
compat ones.
---
 bits/struct_lastlog.h                         |  4 +--
 .../sysv/linux/s390/bits/struct_lastlog.h     | 35 -------------------
 2 files changed, 2 insertions(+), 37 deletions(-)
 delete mode 100644 sysdeps/unix/sysv/linux/s390/bits/struct_lastlog.h
  

Comments

Andreas Schwab July 29, 2020, 9:04 p.m. UTC | #1
s/User/Use/

Andreas.
  
Andreas Schwab July 29, 2020, 9:14 p.m. UTC | #2
On Jul 29 2020, Adhemerval Zanella via Libc-alpha wrote:

> It is not used directly on any symbol, so there is no need to add
> compat ones.

struct lastlog is an external type, describing the format of
/var/log/lastlog.

Andreas.
  
Adhemerval Zanella July 30, 2020, 12:39 p.m. UTC | #3
On 29/07/2020 18:14, Andreas Schwab wrote:
> On Jul 29 2020, Adhemerval Zanella via Libc-alpha wrote:
> 
>> It is not used directly on any symbol, so there is no need to add
>> compat ones.
> 
> struct lastlog is an external type, describing the format of
> /var/log/lastlog.
>

Right, so which would be best way to handle this transition?  My idea
would to be approach to consumers, shadow-utils developers for instance,
and state the desirable to make glibc y2038 proof and why this interfaces
need to be changed.
  
Florian Weimer July 30, 2020, 4:19 p.m. UTC | #4
* Adhemerval Zanella via Libc-alpha:

> On 29/07/2020 18:14, Andreas Schwab wrote:
>> On Jul 29 2020, Adhemerval Zanella via Libc-alpha wrote:
>> 
>>> It is not used directly on any symbol, so there is no need to add
>>> compat ones.
>> 
>> struct lastlog is an external type, describing the format of
>> /var/log/lastlog.
>>
>
> Right, so which would be best way to handle this transition?  My idea
> would to be approach to consumers, shadow-utils developers for instance,
> and state the desirable to make glibc y2038 proof and why this interfaces
> need to be changed.

Sounds reasonable.  We also have a security bug that is related to
direct access to these files:

  <https://sourceware.org/bugzilla/show_bug.cgi?id=24492>

If we need to mediate access for format translation, maybe this can be
addressed at the same time?

Thanks,
Florian
  
Joseph Myers July 30, 2020, 6:54 p.m. UTC | #5
On Thu, 30 Jul 2020, Florian Weimer via Libc-alpha wrote:

> Sounds reasonable.  We also have a security bug that is related to
> direct access to these files:
> 
>   <https://sourceware.org/bugzilla/show_bug.cgi?id=24492>
> 
> If we need to mediate access for format translation, maybe this can be
> addressed at the same time?

Having different filenames for the new format might be safest, which would 
naturally go along with having some form of mediation for access to utmp / 
wtmp / lastlog.
  
Adhemerval Zanella July 30, 2020, 9:46 p.m. UTC | #6
On 30/07/2020 13:19, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>> On 29/07/2020 18:14, Andreas Schwab wrote:
>>> On Jul 29 2020, Adhemerval Zanella via Libc-alpha wrote:
>>>
>>>> It is not used directly on any symbol, so there is no need to add
>>>> compat ones.
>>>
>>> struct lastlog is an external type, describing the format of
>>> /var/log/lastlog.
>>>
>>
>> Right, so which would be best way to handle this transition?  My idea
>> would to be approach to consumers, shadow-utils developers for instance,
>> and state the desirable to make glibc y2038 proof and why this interfaces
>> need to be changed.
> 
> Sounds reasonable.  We also have a security bug that is related to
> direct access to these files:
> 
>   <https://sourceware.org/bugzilla/show_bug.cgi?id=24492>
> 
> If we need to mediate access for format translation, maybe this can be
> addressed at the same time?

I think BZ#24492 is another issue and one that is not easily fixed
without some changes on the utmp-file.c internals.  The utmp/utmpx files 
are read by the ut{x} functions and used on programs such as 'who', and
to fully avoid possible F_SETLKW deadlocks the write must synchronize
using a file not accessible by normal users.

The alarm trick mitigates the deadlock issue, but it does not help
with the fact the log entry is not generated.  The bug report suggestion
of using a extra file to act as the lock for write synchronization might
be a better option.  Another possibility is to move to use a daemon broker 
to actually operate on utmp/utmpx, but I also think it is out of the scope.

I am inclined of using the extra lock file, thoughts?
  
Adhemerval Zanella July 30, 2020, 9:53 p.m. UTC | #7
On 30/07/2020 15:54, Joseph Myers wrote:
> On Thu, 30 Jul 2020, Florian Weimer via Libc-alpha wrote:
> 
>> Sounds reasonable.  We also have a security bug that is related to
>> direct access to these files:
>>
>>   <https://sourceware.org/bugzilla/show_bug.cgi?id=24492>
>>
>> If we need to mediate access for format translation, maybe this can be
>> addressed at the same time?
> 
> Having different filenames for the new format might be safest, which would 
> naturally go along with having some form of mediation for access to utmp / 
> wtmp / lastlog.
> 

The different filename sound slike a strategy and I understand that the idea
would also to use new versions for the utmp/utmpx symbols. What about the 
compatibility symbols, should them be built on top of the new ABI and access
the newer files or use the old format and access the old filenames?

As for lastlog, I think it would be feasible to provide a API to mediate
the file access as a GNU extension.
  
Joseph Myers July 31, 2020, 12:31 a.m. UTC | #8
On Thu, 30 Jul 2020, Adhemerval Zanella via Libc-alpha wrote:

> The different filename sound slike a strategy and I understand that the idea
> would also to use new versions for the utmp/utmpx symbols. What about the 
> compatibility symbols, should them be built on top of the new ABI and access
> the newer files or use the old format and access the old filenames?

I suppose that gets into questions about programs explicitly using 
utmpname.  If a program using the old symbol versions called utmpname 
explicitly with some filename, that's probably the name of a file in the 
old format.  But if it doesn't call utmpname, either you can access the 
old files in the old format and not get current data, or the new files in 
the new format and convert.
  
Lukasz Majewski Oct. 22, 2020, 9:25 a.m. UTC | #9
Hi Adhemerval,

> It is not used directly on any symbol, so there is no need to add
> compat ones.
> ---
>  bits/struct_lastlog.h                         |  4 +--
>  .../sysv/linux/s390/bits/struct_lastlog.h     | 35
> ------------------- 2 files changed, 2 insertions(+), 37 deletions(-)
>  delete mode 100644 sysdeps/unix/sysv/linux/s390/bits/struct_lastlog.h
> 
> diff --git a/bits/struct_lastlog.h b/bits/struct_lastlog.h
> index 122a44abd0..6882015d7c 100644
> --- a/bits/struct_lastlog.h
> +++ b/bits/struct_lastlog.h
> @@ -24,8 +24,8 @@
>     previous logins.  */
>  struct lastlog
>    {
> -#if __WORDSIZE_TIME64_COMPAT32
> -    int32_t ll_time;
> +#if __TIMESIZE != 64
> +    int64_t ll_time;

Maybe __time64_t would fit better here?

>  #else
>      __time_t ll_time;
>  #endif

Reviewed-by: Lukasz Majewski <lukma@denx.de>

> diff --git a/sysdeps/unix/sysv/linux/s390/bits/struct_lastlog.h
> b/sysdeps/unix/sysv/linux/s390/bits/struct_lastlog.h deleted file
> mode 100644 index 2fa409aeec..0000000000
> --- a/sysdeps/unix/sysv/linux/s390/bits/struct_lastlog.h
> +++ /dev/null
> @@ -1,35 +0,0 @@
> -/* The 'struct lastlog' type.
> -   Copyright (C) 2020 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -
> -   The GNU C Library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   The GNU C Library is distributed in the hope that it will be
> useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with the GNU C Library; if not, see
> -   <https://www.gnu.org/licenses/>.  */
> -
> -#ifndef _UTMP_H
> -# error "Never include <bits/struct_lastlog.h> directly; use
> <utmp.h> instead." -#endif
> -
> -/* The structure describing an entry in the database of
> -   previous logins.  */
> -struct lastlog
> -  {
> -#if __WORDSIZE == 32
> -    int64_t ll_time;
> -#else
> -    __time_t ll_time;
> -#endif
> -    char ll_line[UT_LINESIZE];
> -    char ll_host[UT_HOSTSIZE];
> -  };
> -




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
  

Patch

diff --git a/bits/struct_lastlog.h b/bits/struct_lastlog.h
index 122a44abd0..6882015d7c 100644
--- a/bits/struct_lastlog.h
+++ b/bits/struct_lastlog.h
@@ -24,8 +24,8 @@ 
    previous logins.  */
 struct lastlog
   {
-#if __WORDSIZE_TIME64_COMPAT32
-    int32_t ll_time;
+#if __TIMESIZE != 64
+    int64_t ll_time;
 #else
     __time_t ll_time;
 #endif
diff --git a/sysdeps/unix/sysv/linux/s390/bits/struct_lastlog.h b/sysdeps/unix/sysv/linux/s390/bits/struct_lastlog.h
deleted file mode 100644
index 2fa409aeec..0000000000
--- a/sysdeps/unix/sysv/linux/s390/bits/struct_lastlog.h
+++ /dev/null
@@ -1,35 +0,0 @@ 
-/* The 'struct lastlog' type.
-   Copyright (C) 2020 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <https://www.gnu.org/licenses/>.  */
-
-#ifndef _UTMP_H
-# error "Never include <bits/struct_lastlog.h> directly; use <utmp.h> instead."
-#endif
-
-/* The structure describing an entry in the database of
-   previous logins.  */
-struct lastlog
-  {
-#if __WORDSIZE == 32
-    int64_t ll_time;
-#else
-    __time_t ll_time;
-#endif
-    char ll_line[UT_LINESIZE];
-    char ll_host[UT_HOSTSIZE];
-  };
-