On Thu, Feb 20, 2020 at 03:31:56PM -0800, Linus Torvalds wrote:
> On Thu, Feb 20, 2020 at 3:29 PM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> >
> > We do know that caller does not want more than the value it has passed in
> > 'size' argument, though.
>
> Ok, fair enough. That's probably a good way to handle the "allocate in
> the caller".
>
> So then I have no issues with that approach.
Turns out that "nobody uses those for userland destinations after that" is not
quite right - we have this:
int copy_fpstate_to_sigframe(void __user *buf, void __user *buf_fx, int size)
{
struct task_struct *tsk = current;
int ia32_fxstate = (buf != buf_fx);
int ret;
ia32_fxstate &= (IS_ENABLED(CONFIG_X86_32) ||
IS_ENABLED(CONFIG_IA32_EMULATION));
if (!access_ok(buf, size))
return -EACCES;
if (!static_cpu_has(X86_FEATURE_FPU))
return fpregs_soft_get(current, NULL, 0,
sizeof(struct user_i387_ia32_struct), NULL,
(struct _fpstate_32 __user *) buf) ? -1 : 1;
... with fpregs_soft_get() behing the shared helper that does, in turn,
call user_regset_copyout(). OTOH, _that_ sure as hell is "fill local
variable, then copy_to_user()" case.
Sigh... Wish we had a quick way to do something along the lines of
"find all callchains leading to <function> that would not come via
-><method>" - doing that manually stank to high heaven ;-/ And in
cases like that nothing along the lines of "simulate a build" is
practical - call chains are all over arch/*, and config-sensitive
as well (32bit vs. 64bit is only beginning of that fun). Thankfully,
none of those involved token-pasting...
Anyway, one observation that came out of that is that we might
be better off with signature change done first; less boilerplate
that way, contrary to what I expected.
Alternatively, we could introduce a new method, with one-by-one
conversion to it. Hmm...
int (*get2)(struct task_struct *target,
const struct user_regset *regset,
struct membuf to);
returning -E... on error and amount left unfilled on success, perhaps?
That seems to generate decent code and is pretty easy on the instances,
especially if membuf_write() et.al. are made to return to->left...
Things like
static int evr_get(struct task_struct *target, const struct user_regset *regset,
struct membuf to)
{
flush_spe_to_thread(target);
membuf_write(&to, &target->thread.evr, sizeof(target->thread.evr));
BUILD_BUG_ON(offsetof(struct thread_struct, acc) + sizeof(u64) !=
offsetof(struct thread_struct, spefscr));
return membuf_write(&to, &target->thread.acc, sizeof(u64));
}
in place of current
static int evr_get(struct task_struct *target, const struct user_regset *regset,
unsigned int pos, unsigned int count,
void *kbuf, void __user *ubuf)
{
int ret;
flush_spe_to_thread(target);
ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
&target->thread.evr,
0, sizeof(target->thread.evr));
BUILD_BUG_ON(offsetof(struct thread_struct, acc) + sizeof(u64) !=
offsetof(struct thread_struct, spefscr));
if (!ret)
ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
&target->thread.acc,
sizeof(target->thread.evr), -1);
return ret;
}
and
static int vr_get(struct task_struct *target, const struct user_regset *regset,
struct membuf to)
{
union {
elf_vrreg_t reg;
u32 word;
} vrsave;
flush_altivec_to_thread(target);
BUILD_BUG_ON(offsetof(struct thread_vr_state, vscr) !=
offsetof(struct thread_vr_state, vr[32]));
membuf_write(&to, &target->thread.vr_state, 33 * sizeof(vector128));
/*
* Copy out only the low-order word of vrsave.
*/
memset(&vrsave, 0, sizeof(vrsave));
vrsave.word = target->thread.vrsave;
return membuf_write(&to, &vrsave, sizeof(vrsave);
}
instead of
static int vr_get(struct task_struct *target, const struct user_regset *regset,
unsigned int pos, unsigned int count,
void *kbuf, void __user *ubuf)
{
int ret;
flush_altivec_to_thread(target);
BUILD_BUG_ON(offsetof(struct thread_vr_state, vscr) !=
offsetof(struct thread_vr_state, vr[32]));
ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
&target->thread.vr_state, 0,
33 * sizeof(vector128));
if (!ret) {
/*
* Copy out only the low-order word of vrsave.
*/
int start, end;
union {
elf_vrreg_t reg;
u32 word;
} vrsave;
memset(&vrsave, 0, sizeof(vrsave));
vrsave.word = target->thread.vrsave;
start = 33 * sizeof(vector128);
end = start + sizeof(vrsave);
ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf, &vrsave,
start, end);
}
return ret;
}
