[-next,1/2] media/az6027: doing dma on the stack
Commit Message
I changed the dma buffers to use allocated memory instead of stack
memory.
The reason for this is documented in Documentation/DMA-API-HOWTO.txt
under the section: "What memory is DMA'able?" That document was only
added a couple weeks ago and there are still lots of modules which
haven't been corrected yet. Btw. Smatch includes a pretty good test to
find places which use stack memory as a dma buffer. That's how I found
these. (http://smatch.sf.net).
Signed-off-by: Dan Carpenter <error27@gmail.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Comments
Dan Carpenter schrieb:
> I changed the dma buffers to use allocated memory instead of stack
> memory.
>
> The reason for this is documented in Documentation/DMA-API-HOWTO.txt
> under the section: "What memory is DMA'able?" That document was only
> added a couple weeks ago and there are still lots of modules which
> haven't been corrected yet. Btw. Smatch includes a pretty good test to
> find places which use stack memory as a dma buffer. That's how I found
> these. (http://smatch.sf.net).
>
> Signed-off-by: Dan Carpenter <error27@gmail.com>
>
> diff --git a/drivers/media/dvb/dvb-usb/az6027.c b/drivers/media/dvb/dvb-usb/az6027.c
> index 8934788..baaa301 100644
> --- a/drivers/media/dvb/dvb-usb/az6027.c
> +++ b/drivers/media/dvb/dvb-usb/az6027.c
> @@ -417,11 +417,15 @@ static int az6027_ci_read_attribute_mem(struct dvb_ca_en50221 *ca,
> u16 value;
> u16 index;
> int blen;
> - u8 b[12];
> + u8 *b;
>
> if (slot != 0)
> return -EINVAL;
>
> + b = kmalloc(12, GFP_KERNEL);
> + if (!b)
> + return -ENOMEM;
> +
> mutex_lock(&state->ca_mutex);
>
> req = 0xC1;
Hi Dan,
i am not sure if that is the way to go.
iff i understand the code correctly the b[12] seems to overcommit only
blen bytes (not 12) is needed. There must be a cheaper way to send a few bytes
of space to send a command to a device. Perhaps gregKH has a hint ?
re,
wh
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
walter harms wrote:
>
> Dan Carpenter schrieb:
>> I changed the dma buffers to use allocated memory instead of stack
>> memory.
>>
>> The reason for this is documented in Documentation/DMA-API-HOWTO.txt
>> under the section: "What memory is DMA'able?" That document was only
>> added a couple weeks ago and there are still lots of modules which
>> haven't been corrected yet. Btw. Smatch includes a pretty good test to
>> find places which use stack memory as a dma buffer. That's how I found
>> these. (http://smatch.sf.net).
>>
>> Signed-off-by: Dan Carpenter <error27@gmail.com>
>>
>> diff --git a/drivers/media/dvb/dvb-usb/az6027.c b/drivers/media/dvb/dvb-usb/az6027.c
>> index 8934788..baaa301 100644
>> --- a/drivers/media/dvb/dvb-usb/az6027.c
>> +++ b/drivers/media/dvb/dvb-usb/az6027.c
>> @@ -417,11 +417,15 @@ static int az6027_ci_read_attribute_mem(struct dvb_ca_en50221 *ca,
>> u16 value;
>> u16 index;
>> int blen;
>> - u8 b[12];
>> + u8 *b;
>>
>> if (slot != 0)
>> return -EINVAL;
>>
>> + b = kmalloc(12, GFP_KERNEL);
>> + if (!b)
>> + return -ENOMEM;
>> +
>> mutex_lock(&state->ca_mutex);
>>
>> req = 0xC1;
>
>
> Hi Dan,
> i am not sure if that is the way to go.
> iff i understand the code correctly the b[12] seems to overcommit only
> blen bytes (not 12) is needed. There must be a cheaper way to send a few bytes
> of space to send a command to a device. Perhaps gregKH has a hint ?
There is: you can add an array on a device private structure to hold
those memory transfers. For now, I'll add this patch, as it corrects
a real bug.
Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
@@ -417,11 +417,15 @@ static int az6027_ci_read_attribute_mem(struct dvb_ca_en50221 *ca,
u16 value;
u16 index;
int blen;
- u8 b[12];
+ u8 *b;
if (slot != 0)
return -EINVAL;
+ b = kmalloc(12, GFP_KERNEL);
+ if (!b)
+ return -ENOMEM;
+
mutex_lock(&state->ca_mutex);
req = 0xC1;
@@ -438,6 +442,7 @@ static int az6027_ci_read_attribute_mem(struct dvb_ca_en50221 *ca,
}
mutex_unlock(&state->ca_mutex);
+ kfree(b);
return ret;
}
@@ -485,11 +490,15 @@ static int az6027_ci_read_cam_control(struct dvb_ca_en50221 *ca,
u16 value;
u16 index;
int blen;
- u8 b[12];
+ u8 *b;
if (slot != 0)
return -EINVAL;
+ b = kmalloc(12, GFP_KERNEL);
+ if (!b)
+ return -ENOMEM;
+
mutex_lock(&state->ca_mutex);
req = 0xC3;
@@ -510,6 +519,7 @@ static int az6027_ci_read_cam_control(struct dvb_ca_en50221 *ca,
}
mutex_unlock(&state->ca_mutex);
+ kfree(b);
return ret;
}
@@ -556,7 +566,11 @@ static int CI_CamReady(struct dvb_ca_en50221 *ca, int slot)
u16 value;
u16 index;
int blen;
- u8 b[12];
+ u8 *b;
+
+ b = kmalloc(12, GFP_KERNEL);
+ if (!b)
+ return -ENOMEM;
req = 0xC8;
value = 0;
@@ -570,6 +584,7 @@ static int CI_CamReady(struct dvb_ca_en50221 *ca, int slot)
} else{
ret = b[0];
}
+ kfree(b);
return ret;
}
@@ -667,8 +682,11 @@ static int az6027_ci_poll_slot_status(struct dvb_ca_en50221 *ca, int slot, int o
u16 value;
u16 index;
int blen;
- u8 b[12];
+ u8 *b;
+ b = kmalloc(12, GFP_KERNEL);
+ if (!b)
+ return -ENOMEM;
mutex_lock(&state->ca_mutex);
req = 0xC5;
@@ -692,6 +710,7 @@ static int az6027_ci_poll_slot_status(struct dvb_ca_en50221 *ca, int slot, int o
}
mutex_unlock(&state->ca_mutex);
+ kfree(b);
return ret;
}
@@ -943,10 +962,16 @@ static int az6027_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msg[], int n
u16 value;
int length;
u8 req;
- u8 data[256];
+ u8 *data;
+
+ data = kmalloc(256, GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
- if (mutex_lock_interruptible(&d->i2c_mutex) < 0)
+ if (mutex_lock_interruptible(&d->i2c_mutex) < 0) {
+ kfree(data);
return -EAGAIN;
+ }
if (num > 2)
warn("more than 2 i2c messages at a time is not handled yet. TODO.");
@@ -1016,6 +1041,7 @@ static int az6027_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msg[], int n
}
}
mutex_unlock(&d->i2c_mutex);
+ kfree(data);
return i;
}
@@ -1036,8 +1062,14 @@ int az6027_identify_state(struct usb_device *udev,
struct dvb_usb_device_description **desc,
int *cold)
{
- u8 b[16];
- s16 ret = usb_control_msg(udev,
+ u8 *b;
+ s16 ret;
+
+ b = kmalloc(16, GFP_KERNEL);
+ if (!b)
+ return -ENOMEM;
+
+ ret = usb_control_msg(udev,
usb_rcvctrlpipe(udev, 0),
0xb7,
USB_TYPE_VENDOR | USB_DIR_IN,
@@ -1048,7 +1080,7 @@ int az6027_identify_state(struct usb_device *udev,
USB_CTRL_GET_TIMEOUT);
*cold = ret <= 0;
-
+ kfree(b);
deb_info("cold: %d\n", *cold);
return 0;
}