Subject: Re: UVM patch: sys_swapctl split
To: Chuck Silvers <chuq@chuq.com>
From: Emmanuel Dreyfus <manu@netbsd.org>
List: tech-kern
Date: 03/17/2002 23:53:42
> - you shouldn't just blindly malloc() the amount of memory that the
> user asked for, since the user could pass in some really large number
> which could cause the system to panic. a better way to do this would be
> to pass in a pointer to a function with the signature of copyout() to
> uvm_swap_stats(), and call that instead of copyout() or memcpy().
> for sys_swapctl() you'd pass the real copyout(), for your compat version
> you'd pass a new function that would reformat the entry to whatever
> structure you need and copyout() the reformatted version instead.
I just tried to implement this, but it's not that easy, since there are actually
two copyouts that should be handled differently. The second copyout causes the
problem:
error = copyout(sdp->swd_path,
&sep->se_path, sdp->swd_pathlen);
I end up with the alternative of passing two copyout-like functions to
uvm_swap_stats, which is getting ugly, or to limit the malloc size. It's esay to
do it, since we know that we won't need more than the actual number of swap
devices.
Here is the new patch, is it okay for you?
Index: uvm_swap.c
===================================================================
RCS file: /cvsroot/syssrc/sys/uvm/uvm_swap.c,v
retrieving revision 1.60
diff -U2 -r1.60 uvm_swap.c
--- uvm_swap.c 2002/03/09 07:28:20 1.60
+++ uvm_swap.c 2002/03/17 22:44:39
@@ -457,5 +457,5 @@
char userpath[PATH_MAX + 1];
size_t len;
- int count, error, misc;
+ int error, misc;
int priority;
UVMHIST_FUNC("sys_swapctl"); UVMHIST_CALLED(pdhist);
@@ -495,51 +495,13 @@
#endif
) {
- sep = (struct swapent *)SCARG(uap, arg);
- count = 0;
+ misc = MIN(uvmexp.nswapdev, misc);
+ len = sizeof(struct swapent) * misc;
+ sep = (struct swapent *)malloc(len, M_TEMP, M_WAITOK);
- LIST_FOREACH(spp, &swap_priority, spi_swappri) {
- for (sdp = CIRCLEQ_FIRST(&spp->spi_swapdev);
- sdp != (void *)&spp->spi_swapdev && misc-- > 0;
- sdp = CIRCLEQ_NEXT(sdp, swd_next)) {
- /*
- * backwards compatibility for system call.
- * note that we use 'struct oswapent' as an
- * overlay into both 'struct swapdev' and
- * the userland 'struct swapent', as we
- * want to retain backwards compatibility
- * with NetBSD 1.3.
- */
- sdp->swd_ose.ose_inuse =
- btodb((u_int64_t)sdp->swd_npginuse <<
- PAGE_SHIFT);
- error = copyout(&sdp->swd_ose, sep,
- sizeof(struct oswapent));
+ uvm_swap_stats(SCARG(uap, cmd), sep, misc, retval);
+ error = copyout(sep, (void *)SCARG(uap, arg), len);
- /* now copy out the path if necessary */
-#if defined(COMPAT_13)
- if (error == 0 && SCARG(uap, cmd) == SWAP_STATS)
-#else
- if (error == 0)
-#endif
- error = copyout(sdp->swd_path,
- &sep->se_path, sdp->swd_pathlen);
-
- if (error)
- goto out;
- count++;
-#if defined(COMPAT_13)
- if (SCARG(uap, cmd) == SWAP_OSTATS)
- sep = (struct swapent *)
- ((struct oswapent *)sep + 1);
- else
-#endif
- sep++;
- }
- }
-
+ free(sep, M_TEMP);
UVMHIST_LOG(pdhist, "<- done SWAP_STATS", 0, 0, 0, 0);
-
- *retval = count;
- error = 0;
goto out;
}
@@ -716,4 +678,70 @@
UVMHIST_LOG(pdhist, "<- done! error=%d", error, 0, 0, 0);
return (error);
+}
+
+/*
+ * swap_stats: implements swapctl(SWAP_STATS). The function is kept
+ * away from sys_swapctl() in order to allow COMPAT_* swapctl()
+ * emulation to use it directly without going through sys_swapctl().
+ * The problem with using sys_swapctl() there is that it involves
+ * copying the swapent array to the stackgap, and this array's size
+ * is not known at build time. Hence it would not be possible to
+ * ensure it would fit in the stackgap in any wase.
+ */
+void
+uvm_swap_stats(cmd, sep, sec, retval)
+ int cmd;
+ struct swapent *sep;
+ int sec;
+ register_t *retval;
+{
+ struct swappri *spp;
+ struct swapdev *sdp;
+ int count = 0;
+
+ UVMHIST_FUNC("uvm_swap_stats");
+
+ UVMHIST_CALLED(pdhist);
+
+ LIST_FOREACH(spp, &swap_priority, spi_swappri) {
+ for (sdp = CIRCLEQ_FIRST(&spp->spi_swapdev);
+ sdp != (void *)&spp->spi_swapdev && sec-- > 0;
+ sdp = CIRCLEQ_NEXT(sdp, swd_next)) {
+ /*
+ * backwards compatibility for system call.
+ * note that we use 'struct oswapent' as an
+ * overlay into both 'struct swapdev' and
+ * the userland 'struct swapent', as we
+ * want to retain backwards compatibility
+ * with NetBSD 1.3.
+ */
+ sdp->swd_ose.ose_inuse =
+ btodb((u_int64_t)sdp->swd_npginuse <<
+ PAGE_SHIFT);
+ (void)memcpy(sep, &sdp->swd_ose,
+ sizeof(struct oswapent));
+
+ /* now copy out the path if necessary */
+#if defined(COMPAT_13)
+ if (SCARG(uap, cmd) == SWAP_STATS)
+#endif
+ (void)memcpy(&sep->se_path, sdp->swd_path,
+ sdp->swd_pathlen);
+
+ count++;
+#if defined(COMPAT_13)
+ if (SCARG(uap, cmd) == SWAP_OSTATS)
+ sep = (struct swapent *)
+ ((struct oswapent *)sep + 1);
+ else
+#endif
+ sep++;
+ }
+ }
+
+ UVMHIST_LOG(pdhist, "<- done", 0, 0, 0, 0);
+
+ *retval = count;
+ return;
}
Index: uvm_swap.h
===================================================================
RCS file: /cvsroot/syssrc/sys/uvm/uvm_swap.h,v
retrieving revision 1.5
diff -U2 -r1.5 uvm_swap.h
--- uvm_swap.h 2000/01/11 06:57:51 1.5
+++ uvm_swap.h 2002/03/17 22:44:40
@@ -38,4 +38,6 @@
#ifdef _KERNEL
+struct swapent;
+
int uvm_swap_get __P((struct vm_page *, int, int));
int uvm_swap_put __P((int, struct vm_page **, int, int));
@@ -43,4 +45,6 @@
void uvm_swap_free __P((int, int));
void uvm_swap_markbad __P((int, int));
+void uvm_swap_stats __P((int, struct swapent *,
+ int, register_t *));
#endif /* _KERNEL */
--
Emmanuel Dreyfus.
JavaScript est encapsule dans HTML, qui encapsulait
deja pas mal d'autres conneries comme ca.
manu@netbsd.org