Date: Thu, 15 Dec 2011 16:23:32 GMT From: Dmitry Afanasiev <KOT@MATPOCKuH.Ru> To: freebsd-gnats-submit@FreeBSD.org Subject: ports/163317: www/oops is not safe on 64bit architectures Message-ID: <201112151623.pBFGNW7O042389@red.freebsd.org> Resent-Message-ID: <201112151630.pBFGU9ea041573@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 163317
>Category: ports
>Synopsis: www/oops is not safe on 64bit architectures
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: freebsd-ports-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Thu Dec 15 16:30:08 UTC 2011
>Closed-Date:
>Last-Modified:
>Originator: Dmitry Afanasiev
>Release: FreeBSD 9.0-CURRENT sparc64
>Organization:
>Environment:
FreeBSD sunrise.elcom.spb.ru 9.0-CURRENT FreeBSD 9.0-CURRENT #23: Wed Jul 6 23:12:11 MSD 2011 root@sunrise:/usr/obj/usr/src/sys/sunrise sparc64
>Description:
www/oops port have a big problems on 64bit architectures:
While building it have many warning about casts from int (4 bytes) to pointer (on 64bit - 8bytes):
run.c:489: warning: cast to pointer from integer of different size
icp.c:340: warning: cast to pointer from integer of different size
parser.y:910: warning: cast from pointer to integer of different size
As result oops works unstable and sometimes unexpectedly crashes, because it stores pointers in int variables, and after them uses broken values from int variables as pointers.
Part of problems is related to bundled regexp library, and can be solved by using regexp library from system.
I propose a patch that fixes the root cause of warning.
Also I dummy replaced type for some variables from int to long. It should not be a problem on 32bit architectures, but may be patch must be more adaptive.
Patched oops works on my system, but I'm not sure that I was able to fix all the problems.
Probably the right decision will have to set this port as broken on 64bit systems.
>How-To-Repeat:
On *64*bit system:
cd /usr/ports/www/oops && make 2>&1 | grep warning
>Fix:
Patch attached with submission follows:
diff -ruN oops.orig/Makefile oops/Makefile
--- oops.orig/Makefile 2011-12-15 18:50:35.301408409 +0400
+++ oops/Makefile 2011-12-15 18:51:47.908412080 +0400
@@ -27,7 +27,8 @@
--localstatedir=${OOPSVAR} \
--libdir=${PREFIX}/libexec/oops \
--enable-oops-user=oops \
- --enable-large-files
+ --enable-large-files \
+ --with-regexp=system
CONFIGURE_ENV+= PTHREAD_CFLAGS="${PTHREAD_CFLAGS}" \
PTHREAD_LIBS="${PTHREAD_LIBS}"
CFLAGS+= -fPIC
diff -ruN oops.orig/files/patch-x64 oops/files/patch-x64
--- oops.orig/files/patch-x64 1970-01-01 03:00:00.000000000 +0300
+++ oops/files/patch-x64 2011-12-15 19:52:10.023408730 +0400
@@ -0,0 +1,222 @@
+diff -ruN src.orig/hash.c src/hash.c
+--- src.orig/hash.c 2011-12-15 19:50:59.084410154 +0400
++++ src/hash.c 2011-12-15 19:27:15.384413000 +0400
+@@ -1,4 +1,5 @@
+ #include <stdio.h>
++#include <string.h>
+ #include <strings.h>
+ #include "hash.h"
+
+@@ -37,7 +38,7 @@
+ int
+ hash_put(hash_t *hash, void *key, void *data, hash_entry_t **res)
+ {
+-unsigned int index = 0;
++unsigned long index = 0;
+ hash_entry_t *he = NULL;
+ hash_row_t *row = NULL;
+ int rc = 0;
+@@ -48,7 +49,7 @@
+ if ( hash->valid != HASH_VALID ) return(EINVAL);
+ switch(hash->type) {
+ case HASH_KEY_INT:
+- index = (unsigned int)key % hash->rows;
++ index = (unsigned long)key % hash->rows;
+ break;
+ case HASH_KEY_STRING:
+ index = string_hash((char*)key) % hash->rows;
+@@ -65,7 +66,7 @@
+ while ( he ) {
+ switch(hash->type) {
+ case(HASH_KEY_INT):
+- if ( (int)he->key == (int)key ) {
++ if ( he->key == key ) {
+ rc = EEXIST;
+ goto done;
+ }
+@@ -122,7 +123,7 @@
+ int
+ hash_get(hash_t *hash, void *key, hash_entry_t **he_res)
+ {
+-unsigned int index = 0;
++unsigned long index = 0;
+ hash_entry_t *he = NULL;
+ hash_row_t *row = NULL;
+ int rc = 0;
+@@ -134,7 +135,7 @@
+ if ( hash->valid != HASH_VALID ) return(EINVAL);
+ switch(hash->type) {
+ case HASH_KEY_INT:
+- index = (unsigned int)key % hash->rows;
++ index = (unsigned long)key % hash->rows;
+ break;
+ case HASH_KEY_STRING:
+ index = string_hash((char*)key) % hash->rows;
+@@ -155,7 +156,7 @@
+ }
+ switch(hash->type) {
+ case(HASH_KEY_INT):
+- if ( (int)he->key == (int)key ) {
++ if ( he->key == key ) {
+ he->ref_count++;
+ *he_res = he;
+ goto done;
+diff -ruN src.orig/icp.c src/icp.c
+--- src.orig/icp.c 2011-12-15 19:50:59.097408824 +0400
++++ src/icp.c 2011-12-15 19:23:18.626418000 +0400
+@@ -36,7 +36,7 @@
+ int version:8;
+ short msg_len;
+ } w0;
+- int rq_n;
++ long rq_n;
+ int opt;
+ int opt_data;
+ int sender;
+@@ -45,15 +45,15 @@
+ struct icp_lookup {
+ struct sockaddr_in sa;
+ char type;
+- int rq_n;
++ long rq_n;
+ };
+
+ static struct peer *peer_by_addr(struct sockaddr_in*);
+ static int process_hit(struct icp_lookup *icp_lookup, struct icp_queue_elem *qe);
+ static void process_icp_msg(int so, char *buf, int len, struct sockaddr_in *, struct sockaddr_in *);
+ static int process_miss(struct icp_lookup *icp_lookup, struct icp_queue_elem *qe);
+-static void send_icp_op(int so, struct sockaddr_in *sa, int op, int rq_n, char *urlp);
+-static void send_icp_op_err(int so, struct sockaddr_in *sa, int rq_n);
++static void send_icp_op(int so, struct sockaddr_in *sa, int op, long rq_n, char *urlp);
++static void send_icp_op_err(int so, struct sockaddr_in *sa, long rq_n);
+
+
+ #define icp_opcode icp_hdr->w0.opcode
+@@ -473,7 +473,7 @@
+ }
+
+ static void
+-send_icp_op_err(int so, struct sockaddr_in *sa, int rq_n)
++send_icp_op_err(int so, struct sockaddr_in *sa, long rq_n)
+ {
+ char buf[5*4];
+ int len = sizeof(buf);
+@@ -488,7 +488,7 @@
+ }
+
+ static void
+-send_icp_op(int so, struct sockaddr_in *sa, int op, int rq_n, char *urlp)
++send_icp_op(int so, struct sockaddr_in *sa, int op, long rq_n, char *urlp)
+ {
+ char *buf;
+ int len = strlen(urlp)+1 + sizeof(struct icp_hdr);
+@@ -545,7 +545,7 @@
+ static int
+ process_miss(struct icp_lookup *icp_lookup, struct icp_queue_elem *qe)
+ {
+-int rq_n;
++long rq_n;
+
+ if ( !qe || !icp_lookup ) return(0);
+ rq_n = icp_lookup->rq_n;
+@@ -578,7 +578,7 @@
+ static int
+ process_hit(struct icp_lookup *icp_lookup, struct icp_queue_elem *qe)
+ {
+-int rq_n;
++long rq_n;
+
+ if ( !qe || !icp_lookup ) return(0);
+ rq_n = icp_lookup->rq_n;
+diff -ruN src.orig/oops.h src/oops.h
+--- src.orig/oops.h 2011-12-15 19:50:59.092410511 +0400
++++ src/oops.h 2011-12-15 19:32:49.781410000 +0400
+@@ -839,8 +839,8 @@
+ struct acls *http;
+ struct acls *icp;
+ struct badports *badports;
+- int bandwidth;
+- int miss_deny; /* TRUE if deny */
++ long bandwidth;
++ long miss_deny; /* TRUE if deny */
+ struct l_string_list *auth_mods; /* auth modules */
+ l_mod_call_list_t *redir_mods; /* redir modules */
+ pthread_mutex_t group_mutex;
+@@ -852,10 +852,10 @@
+ struct denytime *denytimes;
+ hash_t *dstdomain_cache; /* cashe for dstdom checks */
+ acl_chk_list_hdr_t *networks_acl;
+- int maxreqrate; /* max request rate */
+- int per_sess_bw; /* max bandw per session */
+- int per_ip_bw; /* bandw per ip address (or client) */
+- int per_ip_conn; /* max number of conns per ip */
++ long maxreqrate; /* max request rate */
++ long per_sess_bw; /* max bandw per session */
++ long per_ip_bw; /* bandw per ip address (or client) */
++ long per_ip_conn; /* max number of conns per ip */
+ struct sockaddr_in conn_from_sa; /* connect from address */
+ };
+
+@@ -922,7 +922,7 @@
+ /* url in icp format */
+ char *url;
+ /* request number - id of request */
+- int rq_n;
++ long rq_n;
+ /* how much peers was sent request to *
+ * (we will wait that many answers) */
+ int requests_sent;
+@@ -1014,7 +1014,7 @@
+ #define IS_HUPED(a) (((a)->answer)&FD_POLL_HU)
+
+ struct pollarg {
+- int fd;
++ long fd;
+ short request;
+ short answer;
+ };
+diff -ruN src.orig/parser.y src/parser.y
+--- src.orig/parser.y 2011-12-15 19:50:59.089409966 +0400
++++ src/parser.y 2011-12-15 19:45:36.266416000 +0400
+@@ -69,7 +69,7 @@
+ %}
+
+ %union {
+- int INT;
++ long INT;
+ char *STRPTR;
+ char CHAR;
+ struct cidr_net *NETPTR;
+@@ -907,26 +907,26 @@
+ new_grp->badports = ops->val;
+ break;
+ case OP_BANDWIDTH:
+- new_grp->bandwidth = (int)ops->val;
++ new_grp->bandwidth = (long)ops->val;
+ break;
+ case OP_PER_SESS_BW:
+- new_grp->per_sess_bw = (int)ops->val;
++ new_grp->per_sess_bw = (long)ops->val;
+ break;
+ case OP_PER_IP_BW:
+- new_grp->per_ip_bw = (int)ops->val;
++ new_grp->per_ip_bw = (long)ops->val;
+ break;
+ case OP_PER_IP_CONN:
+- new_grp->per_ip_conn = (int)ops->val;
++ new_grp->per_ip_conn = (long)ops->val;
+ break;
+ case OP_CONN_FROM:
+ memcpy(&new_grp->conn_from_sa, ops->val, sizeof(new_grp->conn_from_sa));
+ free(ops->val);
+ break;
+ case OP_MISS:
+- new_grp->miss_deny = (int)ops->val;
++ new_grp->miss_deny = (long)ops->val;
+ break;
+ case OP_MAXREQRATE:
+- new_grp->maxreqrate = (int)ops->val;
++ new_grp->maxreqrate = (long)ops->val;
+ break;
+ case OP_AUTH_MODS:
+ if ( ops->val ) {
>Release-Note:
>Audit-Trail:
>Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201112151623.pBFGNW7O042389>