josh at joshtriplett.org
2014-Oct-29 17:17 UTC
[PATCH v3 3/3] x86: Support compiling out userspace I/O (iopl and ioperm)
On Wed, Oct 29, 2014 at 09:59:25AM -0700, Kees Cook wrote:> On Wed, Oct 29, 2014 at 9:10 AM, Josh Triplett <josh at joshtriplett.org> wrote: > > --- a/arch/x86/kernel/process-io.h > > +++ b/arch/x86/kernel/process-io.h > > @@ -1,9 +1,17 @@ > > #ifndef _X86_KERNEL_PROCESS_IO_H > > #define _X86_KERNEL_PROCESS_IO_H > > > > +static inline void clear_thread_io_bitmap(struct task_struct *p) > > +{ > > +#ifdef CONFIG_X86_IOPORT > > + p->thread.io_bitmap_ptr = NULL; > > +#endif /* CONFIG_X86_IOPORT */ > > +} > > Personally, I prefer seeing these kinds of optional functions declared > in a single block rather than having the #ifdefs inside the functions: > > #ifdef CONFIG_X86_IOPORT > static inline void clear_thread_io_bitmap(struct task_struct *p) > { > ... > } > > static inline int copy_io_bitmap(struct task_struct *me, > struct task_struct *p) > { > ... > } > > ...remaining_functions... > > #else > static inline void clear_thread_io_bitmap(struct task_struct *p) { } > static inline int copy_io_bitmap(struct task_struct *me, > struct task_struct *p) > { > return 0; > } > ...remaining functions... > #endif /* CONFIG_X86_IOPORT */ > > But this is entirely a style decision, so I leave it up to the x86 > maintainers ...I can certainly do that if the x86 maintainers prefer, but that tends to produce a net increase in lines of code, as well as duplicating all the function prototypes, which to me seems more error-prone. If the stub versions contained any code, rather than just becoming no-ops, I'd definitely do that.> Another nit may be that we should call this CONFIG_SYSCALL_IOPL or > CONFIG_SYSCALL_IOPERM in keeping with the other CONFIG_SYSCALL_* > naming thread? Again, I don't really care strongly beyond really > wanting to use this new feature! :)I don't feel strongly about the naming. Ingo?> Thanks for working on this!No problem. I look forward to seeing it used, in Chrome OS and elsewhere. :) - Josh Triplett
H. Peter Anvin
2014-Oct-29 17:20 UTC
[PATCH v3 3/3] x86: Support compiling out userspace I/O (iopl and ioperm)
On 10/29/2014 10:17 AM, josh at joshtriplett.org wrote:>> >> But this is entirely a style decision, so I leave it up to the x86 >> maintainers ... > > I can certainly do that if the x86 maintainers prefer, but that tends to > produce a net increase in lines of code, as well as duplicating all the > function prototypes, which to me seems more error-prone. If the > stub versions contained any code, rather than just becoming no-ops, I'd > definitely do that. >I concur with this style choice.>> Another nit may be that we should call this CONFIG_SYSCALL_IOPL or >> CONFIG_SYSCALL_IOPERM in keeping with the other CONFIG_SYSCALL_* >> naming thread? Again, I don't really care strongly beyond really >> wanting to use this new feature! :) > > I don't feel strongly about the naming. Ingo?It is sort of a special case here, as this reflects more than one syscall. -hpa
josh at joshtriplett.org
2014-Oct-29 17:58 UTC
[PATCH v3 3/3] x86: Support compiling out userspace I/O (iopl and ioperm)
On Wed, Oct 29, 2014 at 10:20:28AM -0700, H. Peter Anvin wrote:> On 10/29/2014 10:17 AM, josh at joshtriplett.org wrote: > >> > >> But this is entirely a style decision, so I leave it up to the x86 > >> maintainers ... > > > > I can certainly do that if the x86 maintainers prefer, but that tends to > > produce a net increase in lines of code, as well as duplicating all the > > function prototypes, which to me seems more error-prone. If the > > stub versions contained any code, rather than just becoming no-ops, I'd > > definitely do that. > > > > I concur with this style choice.To clarify: you concur with Kees's suggested change or with the style I used in my patch?> >> Another nit may be that we should call this CONFIG_SYSCALL_IOPL or > >> CONFIG_SYSCALL_IOPERM in keeping with the other CONFIG_SYSCALL_* > >> naming thread? Again, I don't really care strongly beyond really > >> wanting to use this new feature! :) > > > > I don't feel strongly about the naming. Ingo? > > It is sort of a special case here, as this reflects more than one syscall.As well as four VT ioctls. :) - Josh Triplett
Possibly Parallel Threads
- [PATCH v3 3/3] x86: Support compiling out userspace I/O (iopl and ioperm)
- [PATCH v3 3/3] x86: Support compiling out userspace I/O (iopl and ioperm)
- [PATCH v3 3/3] x86: Support compiling out userspace I/O (iopl and ioperm)
- [PATCH v3 3/3] x86: Support compiling out userspace I/O (iopl and ioperm)
- [PATCH v3 3/3] x86: Support compiling out userspace I/O (iopl and ioperm)