On Sat, Sep 25, 2010 at 11:10:53PM +0200, Sergio Gelato wrote:
> * Simon Horman [2010-09-25 21:34:02 +0900]:
> > On Fri, Sep 24, 2010 at 10:36:09AM +0200, Sergio Gelato wrote:
> > > The main problem is that perdition/io.c:io_pipe() and its caller
> > > perdition/perdition.c:perdition_log_close() use int counters
> > > while the corresponding arguments in vanessa_socket_pipe_func()
> > > are declared size_t. I'd worry about stack corruption on platforms
> > > where sizeof(size_t) > sizeof(int).
> > >
> > > Suggested fix: declare those counters size_t, and (for cosmetic purposes)
> > > cast them to unsigned long before formatting them with %lu instead of %d.
> >
> > Thanks, I'll fix that.
> >
> > Do you think it warrants an update to the testing (= already frozen squeeze)
> > package?
>
> I do: amd64 has sizeof(size_t)==8 and sizeof(int)==4.
Hi,
could you take a look at the following patch and see what you think?
I have isolated three integer/size_t with problems.
1) perdition_log_close() takes integer arguments but the counters are
actually size_t.
I think that the resulting bug is cosmetic.
2) __io_pipe_read() and __io_pipe_write() return an int but
the counter in question is a ssize_t.
I actually don't think this can manifest in a problem as the maximum
return value is limited by the count argument, which is 1024
(=BUFFER_SIZE). If count was greater than 2^31 then a problem could
occur whereby a large read was mis-read as an error and the connection
would be prematurely closed.
3) perditiondb_odbc.c:dbserver_get() passes the address of
an SQLINTEGER instead of the address of a SQLLEN to the
odbc library call SQLBindCol() twice.
This seems like it could cause a stack corruption on
systems such as amd64 where SQLINTEGER (= typdef int) is 4 bytes wide
but SQLLEN (= typdef long) is 8 bytes wide.
Index: perdition/perdition/perdition.c
===================================================================
--- perdition.orig/perdition/perdition.c 2010-09-26 15:42:17.000000000
+0900
+++ perdition/perdition/perdition.c 2010-09-26 15:42:30.000000000 +0900
@@ -309,14 +309,14 @@ static void perdition_log_close_early(co
}
static void perdition_log_close(const char *from_to_host_str,
- struct auth *auth, int received, int sent)
+ struct auth *auth, size_t received, size_t sent)
{
struct quoted_str authorisation_id = quote_str(auth->authorisation_id);
VANESSA_LOGGER_ERR_UNSAFE("Closing session:%s "
"authorisation_id=%s%s%s "
"authentication_id=\"%s\" "
- "received=%d sent=%d",
+ "received=%zu sent=%zu",
from_to_host_str,
authorisation_id.quote,
authorisation_id.str,
Index: perdition/perdition/io.c
===================================================================
--- perdition.orig/perdition/io.c 2010-09-26 15:42:17.000000000 +0900
+++ perdition/perdition/io.c 2010-09-26 15:55:41.000000000 +0900
@@ -655,7 +655,7 @@ err:
* 0 otherwise (one of io_a or io_b closes gracefully)
**********************************************************************/
-static int __io_pipe_read(int fd, void *buf, size_t count, void *data){
+static ssize_t __io_pipe_read(int fd, void *buf, size_t count, void *data){
io_t *io;
io_select_t *s;
ssize_t bytes;
@@ -693,7 +693,9 @@ err:
}
-static int __io_pipe_write(int fd, const void *buf, size_t count, void *data){
+static ssize_t
+__io_pipe_write(int fd, const void *buf, size_t count, void *data)
+{
io_t *io;
io_select_t *s;
Index: perdition/perdition/db/odbc/perditiondb_odbc.c
===================================================================
--- perdition.orig/perdition/db/odbc/perditiondb_odbc.c 2010-09-26
15:42:17.000000000 +0900
+++ perdition/perdition/db/odbc/perditiondb_odbc.c 2010-09-26
15:42:30.000000000 +0900
@@ -214,7 +214,7 @@ int dbserver_get(const char *key_str, co
char **str_return, size_t * len_return)
{
SQLINTEGER rc;
- SQLINTEGER rc2;
+ SQLLEN rc2;
int status = -1;
SQLHENV env;
SQLHDBC hdbc;
--
To UNSUBSCRIBE, email to [email protected]
with a subject of "unsubscribe". Trouble? Contact [email protected]
