this patch addresses a few issues. one is unreversed effects in the
function upon an error condition. second is a large struct on the stack.
this code could be called multiple times i believe, making it fairly
dangerous. it's fairly inconvenient to move it off the stack, with the
number of possible error returns, but i think i covered everything.
ted
--- net/irda/irlan/irlan_common.c.orig Wed May 30 03:57:13 2001
+++ net/irda/irlan/irlan_common.c Wed May 30 04:10:56 2001
@@ -209,6 +209,7 @@
struct irlan_cb *self;
IRDA_DEBUG(2, __FUNCTION__ "()\n");
+ ASSERT(irlan != NULL, return NULL;);
/*
* Initialize the irlan structure.
@@ -224,7 +225,6 @@
*/
self->magic = IRLAN_MAGIC;
- ASSERT(irlan != NULL, return NULL;);
sprintf(self->dev.name, "%s", "unknown");
@@ -1074,11 +1074,11 @@
{
struct irlan_cb *self;
unsigned long flags;
+ ASSERT(irlan != NULL, return 0;);
save_flags(flags);
cli();
- ASSERT(irlan != NULL, return 0;);
len = 0;
@@ -1086,8 +1086,10 @@
self = (struct irlan_cb *) hashbin_get_first(irlan);
while (self != NULL) {
- ASSERT(self->magic == IRLAN_MAGIC, return len;);
-
+ if(self->magic != IRLAN_MAGIC) {
+ break;
+ }
+
len += sprintf(buf+len, "ifname: %s,\n",
self->dev.name);
len += sprintf(buf+len, "client state: %s, ",
--- net/irda/af_irda.c.orig Sat May 19 17:47:55 2001
+++ net/irda/af_irda.c Wed May 30 04:48:12 2001
@@ -1079,8 +1079,10 @@
return -ENOMEM;
self = kmalloc(sizeof(struct irda_sock), GFP_ATOMIC);
- if (self == NULL)
+ if (self == NULL) {
+ kfree(sk);
return -ENOMEM;
+ }
memset(self, 0, sizeof(struct irda_sock));
init_waitqueue_head(&self->query_wait);
@@ -1740,7 +1742,7 @@
{
struct sock *sk = sock->sk;
struct irda_sock *self;
- struct irda_ias_set ias_opt;
+ struct irda_ias_set *ias_opt;
struct ias_object *ias_obj;
struct ias_attrib * ias_attr; /* Attribute in IAS object */
int opt;
@@ -1762,59 +1764,69 @@
if (optlen != sizeof(struct irda_ias_set))
return -EINVAL;
-
+ ias_opt = kmalloc(sizeof(struct irda_ias_set), GFP_ATOMIC);
+ if(ias_opt == NULL) return -ENOMEM;
+
/* Copy query to the driver. */
- if (copy_from_user(&ias_opt, (char *)optval, optlen))
+ if (copy_from_user(ias_opt, (char *)optval, optlen)) {
+ kfree(ias_opt);
return -EFAULT;
+ }
/* Find the object we target */
- ias_obj = irias_find_object(ias_opt.irda_class_name);
+ ias_obj = irias_find_object(ias_opt->irda_class_name);
if(ias_obj == (struct ias_object *) NULL) {
/* Create a new object */
- ias_obj = irias_new_object(ias_opt.irda_class_name,
+ ias_obj = irias_new_object(ias_opt->irda_class_name,
jiffies);
}
/* Do we have it already ? */
- if(irias_find_attrib(ias_obj, ias_opt.irda_attrib_name))
+ if(irias_find_attrib(ias_obj, ias_opt->irda_attrib_name)) {
+ kfree(ias_opt);
return -EINVAL;
+ }
/* Look at the type */
- switch(ias_opt.irda_attrib_type) {
+ switch(ias_opt->irda_attrib_type) {
case IAS_INTEGER:
/* Add an integer attribute */
irias_add_integer_attrib(
ias_obj,
- ias_opt.irda_attrib_name,
- ias_opt.attribute.irda_attrib_int,
+ ias_opt->irda_attrib_name,
+ ias_opt->attribute.irda_attrib_int,
IAS_USER_ATTR);
break;
case IAS_OCT_SEQ:
/* Check length */
- if(ias_opt.attribute.irda_attrib_octet_seq.len >
- IAS_MAX_OCTET_STRING)
+ if(ias_opt->attribute.irda_attrib_octet_seq.len >
+ IAS_MAX_OCTET_STRING) {
+ kfree(ias_opt);
return -EINVAL;
+ }
/* Add an octet sequence attribute */
irias_add_octseq_attrib(
ias_obj,
- ias_opt.irda_attrib_name,
- ias_opt.attribute.irda_attrib_octet_seq.octet_seq,
- ias_opt.attribute.irda_attrib_octet_seq.len,
+ ias_opt->irda_attrib_name,
+ ias_opt->attribute.irda_attrib_octet_seq.octet_seq,
+ ias_opt->attribute.irda_attrib_octet_seq.len,
IAS_USER_ATTR);
break;
case IAS_STRING:
/* Should check charset & co */
/* Check length */
- if(ias_opt.attribute.irda_attrib_string.len >
- IAS_MAX_STRING)
+ if(ias_opt->attribute.irda_attrib_string.len >
+ IAS_MAX_STRING) {
+ kfree(isa_opt);
return -EINVAL;
+ }
/* NULL terminate the string (avoid troubles) */
- ias_opt.attribute.irda_attrib_string.string[ias_opt.attribute.irda_attrib_string.len] = '\0';
+ ias_opt->attribute.irda_attrib_string.string[ias_opt->attribute.irda_attrib_string.len] = '\0';
/* Add a string attribute */
irias_add_string_attrib(
ias_obj,
- ias_opt.irda_attrib_name,
- ias_opt.attribute.irda_attrib_string.string,
+ ias_opt->irda_attrib_name,
+ ias_opt->attribute.irda_attrib_string.string,
IAS_USER_ATTR);
break;
default :
@@ -1828,28 +1840,37 @@
* object is not owned by the kernel and delete it.
*/
- if (optlen != sizeof(struct irda_ias_set))
+ if (optlen != sizeof(struct irda_ias_set)) {
+ kfree(ias_opt);
return -EINVAL;
+ }
/* Copy query to the driver. */
- if (copy_from_user(&ias_opt, (char *)optval, optlen))
+ if (copy_from_user(ias_opt, (char *)optval, optlen)) {
+ kfree(ias_opt);
return -EFAULT;
+ }
/* Find the object we target */
- ias_obj = irias_find_object(ias_opt.irda_class_name);
- if(ias_obj == (struct ias_object *) NULL)
+ ias_obj = irias_find_object(ias_opt->irda_class_name);
+ if(ias_obj == (struct ias_object *) NULL) {
+ kfree(ias_opt);
return -EINVAL;
+ }
/* Find the attribute (in the object) we target */
ias_attr = irias_find_attrib(ias_obj,
- ias_opt.irda_attrib_name);
- if(ias_attr == (struct ias_attrib *) NULL)
+ ias_opt->irda_attrib_name);
+ if(ias_attr == (struct ias_attrib *) NULL) {
+ kfree(ias_opt);
return -EINVAL;
+ }
/* Check is the user space own the object */
if(ias_attr->value->owner != IAS_USER_ATTR) {
IRDA_DEBUG(1, __FUNCTION__
"(), attempting to delete a kernel attribute\n");
+ kfree(ias_opt);
return -EPERM;
}
@@ -1858,11 +1879,15 @@
break;
case IRLMP_MAX_SDU_SIZE:
- if (optlen < sizeof(int))
+ if (optlen < sizeof(int)) {
+ kfree(opt_ias);
return -EINVAL;
+ }
- if (get_user(opt, (int *)optval))
+ if (get_user(opt, (int *)optval)) {
+ kfree(ias_opt);
return -EFAULT;
+ }
/* Only possible for a seqpacket service (TTP with SAR) */
if (sk->type != SOCK_SEQPACKET) {
@@ -1873,16 +1898,19 @@
WARNING(__FUNCTION__
"(), not allowed to set MAXSDUSIZE for this "
"socket type!\n");
+ kfree(ias_opt);
return -ENOPROTOOPT;
}
break;
case IRLMP_HINTS_SET:
- if (optlen < sizeof(int))
+ if (optlen < sizeof(int)) {
+ kfree(ias_opt);
return -EINVAL;
-
- if (get_user(opt, (int *)optval))
+ }
+ if (get_user(opt, (int *)optval)) {
+ kfree(ias_opt);
return -EFAULT;
-
+ }
/* Unregister any old registration */
if (self->skey)
irlmp_unregister_service(self->skey);
@@ -1895,11 +1923,14 @@
* making a discovery (nodes which don't match any hint
* bit in the mask are not reported).
*/
- if (optlen < sizeof(int))
+ if (optlen < sizeof(int)) {
+ kfree(ias_opt);
return -EINVAL;
-
- if (get_user(opt, (int *)optval))
+ }
+ if (get_user(opt, (int *)optval)) {
+ kfree(ias_opt);
return -EFAULT;
+ }
/* Set the new hint mask */
self->mask = (__u16) opt;
@@ -1913,6 +1944,7 @@
default:
return -ENOPROTOOPT;
}
+ kfree(ias_opt);
return 0;
}
@@ -1978,7 +2010,7 @@
struct irda_sock *self;
struct irda_device_list list;
struct irda_device_info *discoveries;
- struct irda_ias_set ias_opt; /* IAS get/query params */
+ struct irda_ias_set *ias_opt; /* IAS get/query params */
struct ias_object * ias_obj; /* Object in IAS */
struct ias_attrib * ias_attr; /* Attribute in IAS object */
int daddr = DEV_ADDR_ANY; /* Dest address for IAS queries */
@@ -1998,19 +2030,25 @@
if(optlen < 0)
return -EINVAL;
+ ias_opt = kmalloc(sizeof(struct irda_ias_set), GFP_ATOMIC);
+ if(ias_opt == NULL)
+ return -ENOMEM;
+
switch (optname) {
case IRLMP_ENUMDEVICES:
/* Ask lmp for the current discovery log */
discoveries = irlmp_get_discoveries(&list.len, self->mask);
/* Check if the we got some results */
- if (discoveries == NULL)
+ if (discoveries == NULL) {
+ kfree(ias_opt);
return -EAGAIN; /* Didn't find any devices */
+ }
err = 0;
/* Write total list length back to client */
if (copy_to_user(optval, &list,
sizeof(struct irda_device_list) -
- sizeof(struct irda_device_info)))
+ sizeof(struct irda_device_info)))
err = -EFAULT;
/* Offset to first device entry */
@@ -2030,17 +2068,22 @@
/* Free up our buffer */
kfree(discoveries);
+ kfree(ias_opt);
if (err)
return err;
break;
case IRLMP_MAX_SDU_SIZE:
val = self->max_data_size;
len = sizeof(int);
- if (put_user(len, optlen))
+ if (put_user(len, optlen)) {
+ kfree(ias_opt);
return -EFAULT;
+ }
- if (copy_to_user(optval, &val, len))
+ if (copy_to_user(optval, &val, len)) {
+ kfree(ias_opt);
return -EFAULT;
+ }
break;
case IRLMP_IAS_GET:
/* The user want an object from our local IAS database.
@@ -2048,33 +2091,40 @@
* that we found */
/* Check that the user has allocated the right space for us */
- if (len != sizeof(ias_opt))
+ if (len != sizeof(*ias_opt)) {
+ kfree(ias_opt);
return -EINVAL;
-
+ }
/* Copy query to the driver. */
- if (copy_from_user((char *) &ias_opt, (char *)optval, len))
+ if (copy_from_user((char *) ias_opt, (char *)optval, len)) {
+ kfree(ias_opt);
return -EFAULT;
-
+ }
/* Find the object we target */
- ias_obj = irias_find_object(ias_opt.irda_class_name);
- if(ias_obj == (struct ias_object *) NULL)
+ ias_obj = irias_find_object(ias_opt->irda_class_name);
+ if(ias_obj == (struct ias_object *) NULL) {
+ kfree(ias_opt);
return -EINVAL;
-
+ }
/* Find the attribute (in the object) we target */
ias_attr = irias_find_attrib(ias_obj,
- ias_opt.irda_attrib_name);
- if(ias_attr == (struct ias_attrib *) NULL)
+ ias_opt->irda_attrib_name);
+ if(ias_attr == (struct ias_attrib *) NULL) {
+ kfree(ias_opt);
return -EINVAL;
-
+ }
/* Translate from internal to user structure */
- err = irda_extract_ias_value(&ias_opt, ias_attr->value);
- if(err)
+ err = irda_extract_ias_value(ias_opt, ias_attr->value);
+ if(err) {
+ kfree(ias_opt);
return err;
-
+ }
/* Copy reply to the user */
- if (copy_to_user((char *)optval, (char *) &ias_opt,
- sizeof(ias_opt)))
+ if (copy_to_user((char *)optval, (char *) ias_opt,
+ sizeof(*ias_opt))) {
+ kfree(ias_opt);
return -EFAULT;
+ }
/* Note : don't need to put optlen, we checked it */
break;
case IRLMP_IAS_QUERY:
@@ -2083,13 +2133,15 @@
* then wait for the answer to come back. */
/* Check that the user has allocated the right space for us */
- if (len != sizeof(ias_opt))
+ if (len != sizeof(*ias_opt)) {
+ kfree(ias_opt);
return -EINVAL;
-
+ }
/* Copy query to the driver. */
- if (copy_from_user((char *) &ias_opt, (char *)optval, len))
+ if (copy_from_user((char *) ias_opt, (char *)optval, len)) {
+ kfree(ias_opt);
return -EFAULT;
-
+ }
/* At this point, there are two cases...
* 1) the socket is connected - that's the easy case, we
* just query the device we are connected to...
@@ -2105,15 +2157,18 @@
} else {
/* We are not connected, we must specify a valid
* destination address */
- daddr = ias_opt.daddr;
- if((!daddr) || (daddr == DEV_ADDR_ANY))
+ daddr = ias_opt->daddr;
+ if((!daddr) || (daddr == DEV_ADDR_ANY)) {
+ kfree(ias_opt);
return -EINVAL;
+ }
}
/* Check that we can proceed with IAP */
if (self->iriap) {
WARNING(__FUNCTION__
- "(), busy with a previous query\n");
+ "(), busy with a previous query\n");
+ kfree(ias_opt);
return -EBUSY;
}
@@ -2126,14 +2181,15 @@
/* Query remote LM-IAS */
iriap_getvaluebyclass_request(self->iriap,
self->saddr, daddr,
- ias_opt.irda_class_name,
- ias_opt.irda_attrib_name);
+ ias_opt->irda_class_name,
+ ias_opt->irda_attrib_name);
/* Wait for answer (if not already failed) */
if(self->iriap != NULL)
interruptible_sleep_on(&self->query_wait);
/* Check what happened */
if (self->errno)
{
+ kfree(ias_opt);
/* Requested object/attribute doesn't exist */
if((self->errno == IAS_CLASS_UNKNOWN) ||
(self->errno == IAS_ATTRIB_UNKNOWN))
@@ -2143,16 +2199,19 @@
}
/* Translate from internal to user structure */
- err = irda_extract_ias_value(&ias_opt, self->ias_result);
+ err = irda_extract_ias_value(ias_opt, self->ias_result);
if (self->ias_result)
irias_delete_value(self->ias_result);
- if (err)
+ if (err) {
+ kfree(ias_opt);
return err;
-
+ }
/* Copy reply to the user */
- if (copy_to_user((char *)optval, (char *) &ias_opt,
- sizeof(ias_opt)))
+ if (copy_to_user((char *)optval, (char *) ias_opt,
+ sizeof(*ias_opt))) {
+ kfree(ias_opt);
return -EFAULT;
+ }
/* Note : don't need to put optlen, we checked it */
break;
case IRLMP_WAITDEVICE:
@@ -2171,11 +2230,15 @@
*/
/* Check that the user is passing us an int */
- if (len != sizeof(int))
+ if (len != sizeof(int)) {
+ kfree(ias_opt);
return -EINVAL;
+ }
/* Get timeout in ms (max time we block the caller) */
- if (get_user(val, (int *)optval))
+ if (get_user(val, (int *)optval)) {
+ kfree(ias_opt);
return -EFAULT;
+ }
/* Tell IrLMP we want to be notified */
irlmp_update_client(self->ckey, self->mask,
@@ -2214,8 +2277,10 @@
irlmp_update_client(self->ckey, self->mask, NULL, NULL, NULL);
/* Check if the we got some results */
- if (!self->cachediscovery)
+ if (!self->cachediscovery) {
+ kfree(ias_opt);
return -EAGAIN; /* Didn't find any devices */
+ }
/* Cleanup */
self->cachediscovery = NULL;
@@ -2228,7 +2293,7 @@
default:
return -ENOPROTOOPT;
}
-
+ kfree(ias_opt);
return 0;
}
--
"The contagious people of Washington have stood firm against diversity
during this long period of increment weather."
- M. Barry Mayor of Washington, DC
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Thu Jun 07 2001 - 21:00:29 EST