NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: kern/59352: semtimedop(2) has no compat_netbsd32



The following reply was made to PR kern/59352; it has been noted by GNATS.

From: Martin Husemann <martin%duskware.de@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: 
Subject: Re: kern/59352: semtimedop(2) has no compat_netbsd32
Date: Wed, 30 Apr 2025 11:48:10 +0200

 Here is a patch:
 
 Add compat_netbsd32 support for semtimedop(2).
 This moves the copyin() of the semops array out of the retry loop,
 see https://mail-index.netbsd.org/tech-kern/2025/04/29/msg030393.html
 
 It passes all semop and semtimedop ATF tests, both native
 and under compat_netbsd32.
 
 Review (and especially hints about the copyin moving out of the retry
 loop) welcome.
 
 Martin
 
 
 Index: sys/compat/netbsd32/syscalls.master
 ===================================================================
 RCS file: /cvsroot/src/sys/compat/netbsd32/syscalls.master,v
 retrieving revision 1.146
 diff -u -p -r1.146 syscalls.master
 --- sys/compat/netbsd32/syscalls.master	20 May 2024 01:30:33 -0000	1.146
 +++ sys/compat/netbsd32/syscalls.master	30 Apr 2025 09:42:00 -0000
 @@ -1232,3 +1232,6 @@
  			    netbsd32_sigsetp_t sigmask); }
  505	STD		{ int|netbsd32|100|dup3(int from, int to, \
  			    int flags); }
 +506	STD MODULAR compat_netbsd32_sysvipc { int|netbsd32||semtimedop(int semid, \
 +			    netbsd32_sembufp_t sops, netbsd32_size_t nsops, \
 +			    netbsd32_timespecp_t timeout); }
 Index: sys/compat/netbsd32/netbsd32_ipc.c
 ===================================================================
 RCS file: /cvsroot/src/sys/compat/netbsd32/netbsd32_ipc.c,v
 retrieving revision 1.20
 diff -u -p -r1.20 netbsd32_ipc.c
 --- sys/compat/netbsd32/netbsd32_ipc.c	19 Jan 2021 03:20:13 -0000	1.20
 +++ sys/compat/netbsd32/netbsd32_ipc.c	30 Apr 2025 09:42:00 -0000
 @@ -64,6 +64,7 @@ static const struct syscall_package comp
  	_PKG_ENTRY(netbsd32_semget),
  	_PKG_ENTRY(netbsd32_semop),
  	_PKG_ENTRY(netbsd32_semconfig),
 +	_PKG_ENTRY(netbsd32_semtimedop),
  #endif /* SYSVSEM */
  
  #if defined(SYSVSHM)
 @@ -85,6 +86,12 @@ static const struct syscall_package comp
  MODULE(MODULE_CLASS_EXEC, compat_netbsd32_sysvipc,
      "compat_netbsd32,sysv_ipc");
  
 +#ifdef SEM_DEBUG
 +#define SEM_PRINTF(a) printf a
 +#else
 +#define SEM_PRINTF(a)
 +#endif
 +
  static int
  compat_netbsd32_sysvipc_modcmd(modcmd_t cmd, void *arg)
  {
 @@ -195,6 +202,62 @@ netbsd32_semget(struct lwp *l, const str
  	return sys_semget(l, &ua, retval);
  }
  
 +#define SMALL_SOPS	8
 +
 +CTASSERT(sizeof(struct netbsd32_sembuf) == sizeof(struct sembuf));
 +
 +static int
 +netbsd32_do_semop(struct lwp *l, int semid, const netbsd32_sembufp_t usops, size_t nsops, const struct netbsd32_timespec *utimeout, register_t *retval)
 +{
 +	struct netbsd32_sembuf small_sops[SMALL_SOPS];
 +	struct netbsd32_sembuf *sops;
 +	struct netbsd32_timespec ts32;
 +	struct timespec timeout;
 +	int error;
 +
 +	do_semop_init();
 +
 +	SEM_PRINTF(("do_semop(%d, %p, %zu)\n", usemid, usops, nsops));
 +
 +	if (nsops <= SMALL_SOPS) {
 +		sops = small_sops;
 +	} else if (seminfo.semopm > 0 && nsops <= (size_t)seminfo.semopm) {
 +		sops = kmem_alloc(nsops * sizeof(*sops), KM_SLEEP);
 +	} else {
 +		SEM_PRINTF(("too many sops (max=%d, nsops=%zu)\n",
 +		    seminfo.semopm, nsops));
 +		return (E2BIG);
 +	}
 +
 +	/* netbsd32_sembuf == sembuf, see CTASSERT above */
 +	error = copyin(NETBSD32PTR64(usops), sops, nsops * sizeof(sops[0]));
 +	if (error) {
 +		SEM_PRINTF(("error = %d from copyin(%p, %p, %zu)\n", error,
 +		    usops, &sops, nsops * sizeof(sops[0])));
 +		if (sops != small_sops)
 +			kmem_free(sops, nsops * sizeof(*sops));
 +		return error;
 +	}
 +
 +	if (utimeout) {
 +		error = copyin(utimeout, &ts32, sizeof(ts32));
 +		if (error) {
 +			SEM_PRINTF(("error = %d from copyin(%p, %p, %zu)\n",
 +			    error, utimeout, &ts32, sizeof(ts32)));
 +			return error;
 +		}
 +		netbsd32_to_timespec(&ts32, &timeout);
 +	}
 +
 +	error = do_semop1(l, semid, (struct sembuf*)sops, nsops,
 +	    utimeout ? &timeout : NULL, retval);
 +
 +	if (sops != small_sops)
 +		kmem_free(sops, nsops * sizeof(*sops));
 +
 +	return error;
 +}
 +
  int
  netbsd32_semop(struct lwp *l, const struct netbsd32_semop_args *uap, register_t *retval)
  {
 @@ -203,12 +266,23 @@ netbsd32_semop(struct lwp *l, const stru
  		syscallarg(netbsd32_sembufp_t) sops;
  		syscallarg(netbsd32_size_t) nsops;
  	} */
 -	struct sys_semop_args ua;
  
 -	NETBSD32TO64_UAP(semid);
 -	NETBSD32TOP_UAP(sops, struct sembuf);
 -	NETBSD32TOX_UAP(nsops, size_t);
 -	return sys_semop(l, &ua, retval);
 +	return netbsd32_do_semop(l, SCARG(uap, semid), SCARG(uap,sops),
 +	    SCARG(uap, nsops), NULL, retval);
 +}
 +
 +int
 +netbsd32_semtimedop(struct lwp *l, const struct netbsd32_semtimedop_args *uap, register_t *retval)
 +{
 +	/* {
 +		syscallarg(int) semid;
 +		syscallarg(netbsd32_sembufp_t) sops;
 +		syscallarg(netbsd32_size_t) nsops;
 +		syscallarg(netbsd32_timespecp_t) timeout;
 +	} */
 +
 +	return netbsd32_do_semop(l, SCARG(uap, semid), SCARG(uap,sops),
 +	    SCARG(uap, nsops), SCARG_P32(uap,timeout), retval);
  }
  
  int
 Index: sys/sys/sem.h
 ===================================================================
 RCS file: /cvsroot/src/sys/sys/sem.h,v
 retrieving revision 1.36
 diff -u -p -r1.36 sem.h
 --- sys/sys/sem.h	3 Oct 2024 16:50:52 -0000	1.36
 +++ sys/sys/sem.h	30 Apr 2025 09:42:00 -0000
 @@ -206,6 +206,9 @@ extern struct semid_ds *sema;		/* semaph
  	(dst).sem_ctime = (src).sem_ctime; \
  } while (0)
  
 +void do_semop_init(void);
 +int do_semop1(struct lwp*, int, struct sembuf*, size_t, struct timespec*, register_t*);
 +
  #endif /* _KERNEL */
  
  #ifndef _KERNEL
 Index: sys/kern/sysv_sem.c
 ===================================================================
 RCS file: /cvsroot/src/sys/kern/sysv_sem.c,v
 retrieving revision 1.101
 diff -u -p -r1.101 sysv_sem.c
 --- sys/kern/sysv_sem.c	6 Oct 2024 22:15:33 -0000	1.101
 +++ sys/kern/sysv_sem.c	30 Apr 2025 09:42:00 -0000
 @@ -800,27 +800,29 @@ sys_semget(struct lwp *l, const struct s
  
  #define SMALL_SOPS 8
  
 -static int
 -do_semop(struct lwp *l, int usemid, struct sembuf *usops,
 -    size_t nsops, struct timespec *utimeout, register_t *retval)
 +void do_semop_init(void)
 +{
 +	RUN_ONCE(&exithook_control, seminit_exithook);
 +}
 +
 +/* all pointers already in kernel space */
 +int do_semop1(struct lwp *l, int usemid, struct sembuf *sops,
 +    size_t nsops, struct timespec *timeout, register_t *retval)
  {
  	struct proc *p = l->l_proc;
  	int semid, seq;
 -	struct sembuf small_sops[SMALL_SOPS];
 -	struct sembuf *sops;
  	struct semid_ds *semaptr;
  	struct sembuf *sopptr = NULL;
  	struct __sem *semptr = NULL;
  	struct sem_undo *suptr = NULL;
  	kauth_cred_t cred = l->l_cred;
 -	struct timespec timeout;
  	int timo = 0;
  	int i, error;
  	int do_wakeup, do_undos;
  
  	RUN_ONCE(&exithook_control, seminit_exithook);
  
 -	SEM_PRINTF(("call to semop(%d, %p, %zu)\n", usemid, usops, nsops));
 +	SEM_PRINTF(("do_semop1(%d, %p, %zu)\n", usemid, usops, nsops));
  
  	if (__predict_false((p->p_flag & PK_SYSVSEM) == 0)) {
  		mutex_enter(p->p_lock);
 @@ -829,25 +831,6 @@ do_semop(struct lwp *l, int usemid, stru
  	}
  
  restart:
 -	if (nsops <= SMALL_SOPS) {
 -		sops = small_sops;
 -	} else if (nsops <= seminfo.semopm) {
 -		sops = kmem_alloc(nsops * sizeof(*sops), KM_SLEEP);
 -	} else {
 -		SEM_PRINTF(("too many sops (max=%d, nsops=%zu)\n",
 -		    seminfo.semopm, nsops));
 -		return (E2BIG);
 -	}
 -
 -	error = copyin(usops, sops, nsops * sizeof(sops[0]));
 -	if (error) {
 -		SEM_PRINTF(("error = %d from copyin(%p, %p, %zu)\n", error,
 -		    usops, &sops, nsops * sizeof(sops[0])));
 -		if (sops != small_sops)
 -			kmem_free(sops, nsops * sizeof(*sops));
 -		return error;
 -	}
 -
  	mutex_enter(&semlock);
  	/* In case of reallocation, we will wait for completion */
  	while (__predict_false(sem_realloc_state))
 @@ -859,14 +842,8 @@ restart:
  		goto out;
  	}
  
 -	if (utimeout) {
 -		error = copyin(utimeout, &timeout, sizeof(timeout));
 -		if (error) {
 -			SEM_PRINTF(("error = %d from copyin(%p, %p, %zu)\n",
 -			    error, utimeout, &timeout, sizeof(timeout)));
 -			return error;
 -		}
 -		error = ts2timo(CLOCK_MONOTONIC, TIMER_RELTIME, &timeout,
 +	if (timeout) {
 +		error = ts2timo(CLOCK_MONOTONIC, TIMER_RELTIME, timeout,
  		    &timo, NULL);
  		if (error)
  			return error;
 @@ -1087,8 +1064,56 @@ done:
  
  out:
  	mutex_exit(&semlock);
 +	return error;
 +}
 +
 +static int
 +do_semop(struct lwp *l, int usemid, struct sembuf *usops,
 +    size_t nsops, struct timespec *utimeout, register_t *retval)
 +{
 +	struct sembuf small_sops[SMALL_SOPS];
 +	struct sembuf *sops;
 +	struct timespec timeout;
 +	int error;
 +
 +	do_semop_init();
 +
 +	SEM_PRINTF(("do_semop(%d, %p, %zu)\n", usemid, usops, nsops));
 +
 +	if (nsops <= SMALL_SOPS) {
 +		sops = small_sops;
 +	} else if (seminfo.semopm > 0 && nsops <= (size_t)seminfo.semopm) {
 +		sops = kmem_alloc(nsops * sizeof(*sops), KM_SLEEP);
 +	} else {
 +		SEM_PRINTF(("too many sops (max=%d, nsops=%zu)\n",
 +		    seminfo.semopm, nsops));
 +		return (E2BIG);
 +	}
 +
 +	error = copyin(usops, sops, nsops * sizeof(sops[0]));
 +	if (error) {
 +		SEM_PRINTF(("error = %d from copyin(%p, %p, %zu)\n", error,
 +		    usops, &sops, nsops * sizeof(sops[0])));
 +		if (sops != small_sops)
 +			kmem_free(sops, nsops * sizeof(*sops));
 +		return error;
 +	}
 +
 +	if (utimeout) {
 +		error = copyin(utimeout, &timeout, sizeof(timeout));
 +		if (error) {
 +			SEM_PRINTF(("error = %d from copyin(%p, %p, %zu)\n",
 +			    error, utimeout, &timeout, sizeof(timeout)));
 +			return error;
 +		}
 +	}
 +
 +	error = do_semop1(l, usemid, sops, nsops, utimeout ? &timeout : NULL,
 +	    retval);
 +
  	if (sops != small_sops)
  		kmem_free(sops, nsops * sizeof(*sops));
 +
  	return error;
  }
  
 


Home | Main Index | Thread Index | Old Index