* [PATCH] tty: provide tty_name() even without CONFIG_TTY
@ 2016-04-27 9:56 Arnd Bergmann
2016-04-27 16:20 ` Paul Moore
2016-04-27 21:18 ` Paul Moore
0 siblings, 2 replies; 6+ messages in thread
From: Arnd Bergmann @ 2016-04-27 9:56 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Richard Guy Briggs, Paul Moore, Arnd Bergmann, Peter Hurley,
Rasmus Villemoes, linux-kernel
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] tty: provide tty_name() even without CONFIG_TTY
2016-04-27 9:56 [PATCH] tty: provide tty_name() even without CONFIG_TTY Arnd Bergmann
@ 2016-04-27 16:20 ` Paul Moore
2016-04-27 17:24 ` Arnd Bergmann
2016-04-27 21:18 ` Paul Moore
1 sibling, 1 reply; 6+ messages in thread
From: Paul Moore @ 2016-04-27 16:20 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Greg Kroah-Hartman, Jiri Slaby, Richard Guy Briggs, Peter Hurley,
Rasmus Villemoes, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] tty: provide tty_name() even without CONFIG_TTY
2016-04-27 16:20 ` Paul Moore
@ 2016-04-27 17:24 ` Arnd Bergmann
2016-04-27 18:21 ` Peter Hurley
0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2016-04-27 17:24 UTC (permalink / raw)
To: Paul Moore
Cc: Greg Kroah-Hartman, Jiri Slaby, Richard Guy Briggs, Peter Hurley,
Rasmus Villemoes, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] tty: provide tty_name() even without CONFIG_TTY
2016-04-27 17:24 ` Arnd Bergmann
@ 2016-04-27 18:21 ` Peter Hurley
2016-04-27 19:57 ` Richard Guy Briggs
0 siblings, 1 reply; 6+ messages in thread
From: Peter Hurley @ 2016-04-27 18:21 UTC (permalink / raw)
To: Arnd Bergmann, Paul Moore
Cc: Greg Kroah-Hartman, Jiri Slaby, Richard Guy Briggs,
Rasmus Villemoes, linux-kernel
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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] tty: provide tty_name() even without CONFIG_TTY
2016-04-27 18:21 ` Peter Hurley
@ 2016-04-27 19:57 ` Richard Guy Briggs
0 siblings, 0 replies; 6+ messages in thread
From: Richard Guy Briggs @ 2016-04-27 19:57 UTC (permalink / raw)
To: Peter Hurley
Cc: Arnd Bergmann, Paul Moore, Greg Kroah-Hartman, Jiri Slaby,
Rasmus Villemoes, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tty: provide tty_name() even without CONFIG_TTY
2016-04-27 9:56 [PATCH] tty: provide tty_name() even without CONFIG_TTY Arnd Bergmann
2016-04-27 16:20 ` Paul Moore
@ 2016-04-27 21:18 ` Paul Moore
1 sibling, 0 replies; 6+ messages in thread
From: Paul Moore @ 2016-04-27 21:18 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Greg Kroah-Hartman, Jiri Slaby, Richard Guy Briggs, Peter Hurley,
Rasmus Villemoes, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-04-27 21:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-27 9:56 [PATCH] tty: provide tty_name() even without CONFIG_TTY Arnd Bergmann
2016-04-27 16:20 ` Paul Moore
2016-04-27 17:24 ` Arnd Bergmann
2016-04-27 18:21 ` Peter Hurley
2016-04-27 19:57 ` Richard Guy Briggs
2016-04-27 21:18 ` Paul Moore
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox