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

Groups > linux.kernel > #1677213

Re: [PATCH 8/9] RISC-V: User-facing API

Show all headers | View raw

On Wed, 28 Jun 2017, Palmer Dabbelt wrote:
> +
> +SYSCALL_DEFINE3(sysriscv_cmpxchg32, unsigned long, arg1, unsigned long, arg2,
> +		unsigned long, arg3)
> +{
> +	unsigned long flags;
> +	unsigned long prev;
> +	unsigned int *ptr;
> +	unsigned int err;
> +
> +	ptr = (unsigned int *)arg1;

Errm. Why isn't arg1 a proper pointer type and the arguments arg2/3 u32?

And please give the arguments a proper name, so it's obvious what is what.

SYSCALL_DEFINE3(sysriscv_cmpxchg32, u32 __user *, ptr, u32 new, u32 old)

Hmm?

> +	if (!access_ok(VERIFY_WRITE, ptr, sizeof(unsigned int)))
> +		return -EFAULT;
> +
> +	preempt_disable();
> +	raw_local_irq_save(flags);

Why do you want to disable interrupts here? This is thread context and
accessing user space memory, so the only protection this needs is against
preemption.

> +	err = __get_user(prev, ptr);
> +	if (likely(!err && prev == arg2))
> +		err = __put_user(arg3, ptr);
> +	raw_local_irq_restore(flags);
> +	preempt_enable();
> +
> +	return unlikely(err) ? err : prev;
> +}
> +
> +SYSCALL_DEFINE3(sysriscv_cmpxchg64, unsigned long, arg1, unsigned long, arg2,
> +		unsigned long, arg3)

This one is even worse. How does this implement cmpxchg64 on a 32bit machine?

Answer: Not at all, because arg2 and 3 are 32bit ....

> +{
> +	unsigned long flags;
> +	unsigned long prev;
> +	unsigned int *ptr;
> +	unsigned int err;
> +
> +	ptr = (unsigned int *)arg1;

Type casting to random pointer types makes the code more obvious
and safe, right? What the heck has a int pointer to do with u64?

> +	if (!access_ok(VERIFY_WRITE, ptr, sizeof(unsigned long)))
> +		return -EFAULT;
> +
> +	preempt_disable();
> +	raw_local_irq_save(flags);

Same as above.

> +	err = __get_user(prev, ptr);

Sigh. Type safety is overrated, right?

Thanks,

	tglx

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

RISC-V Linux Port v3 Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-28 21:00 +0200
  [PATCH 6/9] RISC-V: Device, timer, IRQs, and the SBI Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-28 21:00 +0200
    Re: [PATCH 6/9] RISC-V: Device, timer, IRQs, and the SBI Tobias Klauser <tklauser@distanz.ch> - 2017-06-29 10:40 +0200
      Re: [PATCH 6/9] RISC-V: Device, timer, IRQs, and the SBI Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-30 01:00 +0200
        Re: [PATCH 6/9] RISC-V: Device, timer, IRQs, and the SBI Tobias Klauser <tklauser@distanz.ch> - 2017-06-30 10:00 +0200
  [PATCH 4/9] RISC-V: ELF and module implementation Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-28 21:00 +0200
  [PATCH 7/9] RISC-V: Paging and MMU Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-28 21:10 +0200
    Re: [PATCH 7/9] RISC-V: Paging and MMU James Hogan <james.hogan@imgtec.com> - 2017-06-29 01:20 +0200
      Re: [PATCH 7/9] RISC-V: Paging and MMU Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-30 00:20 +0200
  [PATCH 9/9] RISC-V: Build Infastructure Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-28 21:10 +0200
    Re: [PATCH 9/9] RISC-V: Build Infastructure Karsten Merker <merker@debian.org> - 2017-06-28 23:10 +0200
      Re: [PATCH 9/9] RISC-V: Build Infastructure Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-28 23:20 +0200
    Re: [PATCH 9/9] RISC-V: Build Infastructure James Hogan <james.hogan@imgtec.com> - 2017-06-28 23:30 +0200
      Re: [PATCH 9/9] RISC-V: Build Infastructure Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-29 18:40 +0200
    Re: [PATCH 9/9] RISC-V: Build Infastructure James Hogan <james.hogan@imgtec.com> - 2017-06-29 01:00 +0200
      Re: [PATCH 9/9] RISC-V: Build Infastructure Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-30 00:20 +0200
  Re: [PATCH 8/9] RISC-V: User-facing API Thomas Gleixner <tglx@linutronix.de> - 2017-06-29 00:00 +0200
  Re: [PATCH 8/9] RISC-V: User-facing API Thomas Gleixner <tglx@linutronix.de> - 2017-06-29 00:00 +0200
    Re: [PATCH 8/9] RISC-V: User-facing API Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-29 19:30 +0200
  Re: [PATCH 8/9] RISC-V: User-facing API James Hogan <james.hogan@imgtec.com> - 2017-06-29 00:50 +0200
    Re: [PATCH 8/9] RISC-V: User-facing API Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-29 23:50 +0200
  Re: [PATCH 5/9] RISC-V: Task implementation James Hogan <james.hogan@imgtec.com> - 2017-06-29 01:40 +0200
    Re: [PATCH 5/9] RISC-V: Task implementation Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-30 01:00 +0200
  Re: [PATCH 5/9] RISC-V: Task implementation Tobias Klauser <tklauser@distanz.ch> - 2017-06-29 10:30 +0200
    Re: [PATCH 5/9] RISC-V: Task implementation Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-30 01:00 +0200
  Re: [PATCH 1/9] RISC-V: Init and Halt Code Geert Uytterhoeven <geert@linux-m68k.org> - 2017-06-29 11:50 +0200
    Re: [PATCH 1/9] RISC-V: Init and Halt Code Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-30 01:00 +0200

csiph-web