Hi,
On 12/05/14 09:52, Krzysztof Opasiak wrote:
I agree that there was a possible off-by-one-error and thank you for
catching this I have missed it, but maybe it is not the best solution?
When you put '\0' sign to target[nmb-1] you are solving off by one
problem but you always lose the last sign in path so the whole path may
be invalid or point to other file. Maybe it would be a better solution
to alloc USBG_MAX_PATH_LENGTH + 1 array and keep target[nmb] = '\0'?
You mean something like this?
diff --git a/src/usbg.c b/src/usbg.c
index 521513d..3774a6d 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -843,14 +843,14 @@ static int usbg_parse_config_binding(usbg_config
*c, char *bpath,
{
int nmb;
int ret;
- char target[USBG_MAX_PATH_LENGTH];
+ char target[USBG_MAX_PATH_LENGTH + 1];
char *target_name;
const char *instance;
usbg_function_type type;
usbg_function *f;
usbg_binding *b;
- nmb = readlink(bpath, target, sizeof(target));
+ nmb = readlink(bpath, target, sizeof(target - 1));
if (nmb < 0) {
ret = usbg_translate_error(errno);
goto out;
@@ -859,7 +859,7 @@ static int usbg_parse_config_binding(usbg_config *c,
char *bpath,
/* readlink() doesn't add this, so we have to do it manually,
* we need to take care to not overrun as readlink can return
* the maximum buffer and have a off-by-one error */
- target[nmb-1] = '\0';
+ target[nmb] = '\0';
/* Target contains a full path
* but we need only function dir name */
target_name = strrchr(target, '/') + 1;
Make target bigger, avoid that readlink uses the full size and the same
mistake happens with 4097 instead of 4096. Although just adding the byte
seems a bit overkill to me.
Maybe it is just better to read 4095 bytes so we have room for the '\0'.
So only do nmb = readlink(bpath, target, sizeof(target - 1)); As I guess
that the kernel uses a USBG_MAX_PATH_LENGTH of 4096 bytes which includes
'\0'?
Regards,
Philippe
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
