Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1388861 > unrolled thread

[PATCH] tty: provide tty_name() even without CONFIG_TTY

Back to article view | Back to linux.kernel

Contents

  [PATCH] tty: provide tty_name() even without CONFIG_TTY Arnd Bergmann <arnd@arndb.de> - 2016-04-27 12:00 +0200
    Re: [PATCH] tty: provide tty_name() even without CONFIG_TTY Paul Moore <paul@paul-moore.com> - 2016-04-27 18:30 +0200
      Re: [PATCH] tty: provide tty_name() even without CONFIG_TTY Arnd Bergmann <arnd@arndb.de> - 2016-04-27 19:30 +0200
        Re: [PATCH] tty: provide tty_name() even without CONFIG_TTY Peter Hurley <peter@hurleysoftware.com> - 2016-04-27 20:30 +0200
          Re: [PATCH] tty: provide tty_name() even without CONFIG_TTY Richard Guy Briggs <rgb@redhat.com> - 2016-04-27 22:00 +0200
    Re: [PATCH] tty: provide tty_name() even without CONFIG_TTY Paul Moore <paul@paul-moore.com> - 2016-04-27 23:20 +0200

#1388861 — [PATCH] tty: provide tty_name() even without CONFIG_TTY

The audit subsystem just started printing the name of the tty,
but that causes a build failure when CONFIG_TTY is disabled:

kernel/built-in.o: In function `audit_log_task_info':
memremap.c:(.text+0x5e34c): undefined reference to `tty_name'
kernel/built-in.o: In function `audit_set_loginuid':
memremap.c:(.text+0x63b34): undefined reference to `tty_name'

This adds tty_name() to the list of functions that are provided
as trivial stubs in that configuration.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: db0a6fb5d97a ("audit: add tty field to LOGIN event")
---
 include/linux/tty.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/tty.h b/include/linux/tty.h
index 3b09f235db66..17b247c94440 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -371,6 +371,7 @@ extern void proc_clear_tty(struct task_struct *p);
 extern struct tty_struct *get_current_tty(void);
 /* tty_io.c */
 extern int __init tty_init(void);
+extern const char *tty_name(const struct tty_struct *tty);
 #else
 static inline void console_init(void)
 { }
@@ -391,6 +392,8 @@ static inline struct tty_struct *get_current_tty(void)
 /* tty_io.c */
 static inline int __init tty_init(void)
 { return 0; }
+static inline const char *tty_name(const struct tty_struct *tty)
+{ return "(none)"; }
 #endif
 
 extern struct ktermios tty_std_termios;
@@ -415,7 +418,6 @@ static inline struct tty_struct *tty_kref_get(struct tty_struct *tty)
 	return tty;
 }
 
-extern const char *tty_name(const struct tty_struct *tty);
 extern const char *tty_driver_name(const struct tty_struct *tty);
 extern void tty_wait_until_sent(struct tty_struct *tty, long timeout);
 extern int __tty_check_change(struct tty_struct *tty, int sig);
-- 
2.7.0

[toc] | [next] | [standalone]

#1389318

On Wed, Apr 27, 2016 at 5:56 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> The audit subsystem just started printing the name of the tty,
> but that causes a build failure when CONFIG_TTY is disabled:
>
> kernel/built-in.o: In function `audit_log_task_info':
> memremap.c:(.text+0x5e34c): undefined reference to `tty_name'
> kernel/built-in.o: In function `audit_set_loginuid':
> memremap.c:(.text+0x63b34): undefined reference to `tty_name'
>
> This adds tty_name() to the list of functions that are provided
> as trivial stubs in that configuration.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: db0a6fb5d97a ("audit: add tty field to LOGIN event")
> ---
>  include/linux/tty.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Thanks for reporting this and providing a patch; I'll be happy to
merge this into the audit#next branch with commit db0a6fb5d97a but I
have one question (see below).

> diff --git a/include/linux/tty.h b/include/linux/tty.h
> index 3b09f235db66..17b247c94440 100644
> --- a/include/linux/tty.h
> +++ b/include/linux/tty.h
> @@ -371,6 +371,7 @@ extern void proc_clear_tty(struct task_struct *p);
>  extern struct tty_struct *get_current_tty(void);
>  /* tty_io.c */
>  extern int __init tty_init(void);
> +extern const char *tty_name(const struct tty_struct *tty);
>  #else
>  static inline void console_init(void)
>  { }
> @@ -391,6 +392,8 @@ static inline struct tty_struct *get_current_tty(void)
>  /* tty_io.c */
>  static inline int __init tty_init(void)
>  { return 0; }
> +static inline const char *tty_name(const struct tty_struct *tty)
> +{ return "(none)"; }
>  #endif

As it currently stands tty_name() returns "NULL tty" when the passed
tty_struct is NULL while this patch returns "(none)" in the case of
CONFIG_TTY=n; it seems like some consistency might be good, yes?  Or
do you think there is value in differentiating between the two cases?

From an audit point of view, we would prefer if both were "(none)".

-- 
paul moore
www.paul-moore.com

[toc] | [prev] | [next] | [standalone]

#1389398

On Wednesday 27 April 2016 12:20:02 Paul Moore wrote:
> > diff --git a/include/linux/tty.h b/include/linux/tty.h
> > index 3b09f235db66..17b247c94440 100644
> > --- a/include/linux/tty.h
> > +++ b/include/linux/tty.h
> > @@ -371,6 +371,7 @@ extern void proc_clear_tty(struct task_struct *p);
> >  extern struct tty_struct *get_current_tty(void);
> >  /* tty_io.c */
> >  extern int __init tty_init(void);
> > +extern const char *tty_name(const struct tty_struct *tty);
> >  #else
> >  static inline void console_init(void)
> >  { }
> > @@ -391,6 +392,8 @@ static inline struct tty_struct *get_current_tty(void)
> >  /* tty_io.c */
> >  static inline int __init tty_init(void)
> >  { return 0; }
> > +static inline const char *tty_name(const struct tty_struct *tty)
> > +{ return "(none)"; }
> >  #endif
> 
> As it currently stands tty_name() returns "NULL tty" when the passed
> tty_struct is NULL while this patch returns "(none)" in the case of
> CONFIG_TTY=n; it seems like some consistency might be good, yes?  Or
> do you think there is value in differentiating between the two cases?
> 
> From an audit point of view, we would prefer if both were "(none)".

Right, I noticed that the audit code prints "(none)" here while the
tty code prints "NULL tty", and that meant I could not make it behave
the same way as all the existing code. I picked "(none)" because
in case of CONFIG_TTY being disabled that is more logical: it's
not a NULL pointer because something went wrong, but instead the
pointer doesn't matter and we know there is no tty.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1389456

On 04/27/2016 10:24 AM, Arnd Bergmann wrote:
> On Wednesday 27 April 2016 12:20:02 Paul Moore wrote:
>>> diff --git a/include/linux/tty.h b/include/linux/tty.h
>>> index 3b09f235db66..17b247c94440 100644
>>> --- a/include/linux/tty.h
>>> +++ b/include/linux/tty.h
>>> @@ -371,6 +371,7 @@ extern void proc_clear_tty(struct task_struct *p);
>>>  extern struct tty_struct *get_current_tty(void);
>>>  /* tty_io.c */
>>>  extern int __init tty_init(void);
>>> +extern const char *tty_name(const struct tty_struct *tty);
>>>  #else
>>>  static inline void console_init(void)
>>>  { }
>>> @@ -391,6 +392,8 @@ static inline struct tty_struct *get_current_tty(void)
>>>  /* tty_io.c */
>>>  static inline int __init tty_init(void)
>>>  { return 0; }
>>> +static inline const char *tty_name(const struct tty_struct *tty)
>>> +{ return "(none)"; }
>>>  #endif
>>
>> As it currently stands tty_name() returns "NULL tty" when the passed
>> tty_struct is NULL while this patch returns "(none)" in the case of
>> CONFIG_TTY=n; it seems like some consistency might be good, yes?  Or
>> do you think there is value in differentiating between the two cases?
>>
>> From an audit point of view, we would prefer if both were "(none)".
> 
> Right, I noticed that the audit code prints "(none)" here while the
> tty code prints "NULL tty", and that meant I could not make it behave
> the same way as all the existing code. I picked "(none)" because
> in case of CONFIG_TTY being disabled that is more logical: it's
> not a NULL pointer because something went wrong, but instead the
> pointer doesn't matter and we know there is no tty.

Apologies for not having foreseen this in the review.
Arnd's solution looks good to me.

[toc] | [prev] | [next] | [standalone]

#1389538

On 16/04/27, Peter Hurley wrote:
> On 04/27/2016 10:24 AM, Arnd Bergmann wrote:
> > On Wednesday 27 April 2016 12:20:02 Paul Moore wrote:
> >>> diff --git a/include/linux/tty.h b/include/linux/tty.h
> >>> index 3b09f235db66..17b247c94440 100644
> >>> --- a/include/linux/tty.h
> >>> +++ b/include/linux/tty.h
> >>> @@ -371,6 +371,7 @@ extern void proc_clear_tty(struct task_struct *p);
> >>>  extern struct tty_struct *get_current_tty(void);
> >>>  /* tty_io.c */
> >>>  extern int __init tty_init(void);
> >>> +extern const char *tty_name(const struct tty_struct *tty);
> >>>  #else
> >>>  static inline void console_init(void)
> >>>  { }
> >>> @@ -391,6 +392,8 @@ static inline struct tty_struct *get_current_tty(void)
> >>>  /* tty_io.c */
> >>>  static inline int __init tty_init(void)
> >>>  { return 0; }
> >>> +static inline const char *tty_name(const struct tty_struct *tty)
> >>> +{ return "(none)"; }
> >>>  #endif
> >>
> >> As it currently stands tty_name() returns "NULL tty" when the passed
> >> tty_struct is NULL while this patch returns "(none)" in the case of
> >> CONFIG_TTY=n; it seems like some consistency might be good, yes?  Or
> >> do you think there is value in differentiating between the two cases?
> >>
> >> From an audit point of view, we would prefer if both were "(none)".
> > 
> > Right, I noticed that the audit code prints "(none)" here while the
> > tty code prints "NULL tty", and that meant I could not make it behave
> > the same way as all the existing code. I picked "(none)" because
> > in case of CONFIG_TTY being disabled that is more logical: it's
> > not a NULL pointer because something went wrong, but instead the
> > pointer doesn't matter and we know there is no tty.
> 
> Apologies for not having foreseen this in the review.
> Arnd's solution looks good to me.

Thanks for catching this Arnd, this solution looks good to me.

It didn't occur to me that tty could go away, though I should have
noticed that for the prototype for tty_kref_put().

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

[toc] | [prev] | [next] | [standalone]

#1389591

On Wednesday, April 27, 2016 11:56:04 AM Arnd Bergmann wrote:
> The audit subsystem just started printing the name of the tty,
> but that causes a build failure when CONFIG_TTY is disabled:
> 
> kernel/built-in.o: In function `audit_log_task_info':
> memremap.c:(.text+0x5e34c): undefined reference to `tty_name'
> kernel/built-in.o: In function `audit_set_loginuid':
> memremap.c:(.text+0x63b34): undefined reference to `tty_name'
> 
> This adds tty_name() to the list of functions that are provided
> as trivial stubs in that configuration.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: db0a6fb5d97a ("audit: add tty field to LOGIN event")
> ---
>  include/linux/tty.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Applied to the audit#next branch, thanks!

> diff --git a/include/linux/tty.h b/include/linux/tty.h
> index 3b09f235db66..17b247c94440 100644
> --- a/include/linux/tty.h
> +++ b/include/linux/tty.h
> @@ -371,6 +371,7 @@ extern void proc_clear_tty(struct task_struct *p);
>  extern struct tty_struct *get_current_tty(void);
>  /* tty_io.c */
>  extern int __init tty_init(void);
> +extern const char *tty_name(const struct tty_struct *tty);
>  #else
>  static inline void console_init(void)
>  { }
> @@ -391,6 +392,8 @@ static inline struct tty_struct *get_current_tty(void)
>  /* tty_io.c */
>  static inline int __init tty_init(void)
>  { return 0; }
> +static inline const char *tty_name(const struct tty_struct *tty)
> +{ return "(none)"; }
>  #endif
> 
>  extern struct ktermios tty_std_termios;
> @@ -415,7 +418,6 @@ static inline struct tty_struct *tty_kref_get(struct
> tty_struct *tty) return tty;
>  }
> 
> -extern const char *tty_name(const struct tty_struct *tty);
>  extern const char *tty_driver_name(const struct tty_struct *tty);
>  extern void tty_wait_until_sent(struct tty_struct *tty, long timeout);
>  extern int __tty_check_change(struct tty_struct *tty, int sig);

-- 
paul moore
www.paul-moore.com

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web