[Suggestion] drivers/staging/tidspbridge: strcpy and strncpy, src length checking issue.

[Suggestion] drivers/staging/tidspbridge: strcpy and strncpy, src length checking issue.

[Chen Gang]

Hello Omar Ramirez Luna:

  in drivers/staging/tidspbridge/rmgr/proc.c:

    if strlen(drv_datap->base_img) == size, will pass checking (line 397)
    the size is the full length of exec_file (line 382, line 468..469)
    strcpy causes issue: src len is strlen(drv_datap->base_img) + '\0'. (line 400)

    strncpy seems also has issue: need use size instead of strlen(iva_img) + 1. (line 402..403)

  please help to check, thanks.

gchen.


 380 static int get_exec_file(struct cfg_devnode *dev_node_obj,
 381                                 struct dev_object *hdev_obj,
 382                                 u32 size, char *exec_file)
 383 {
 384         u8 dev_type;
 385         s32 len;
 386         struct drv_data *drv_datap = dev_get_drvdata(bridge);
 387 
 388         dev_get_dev_type(hdev_obj, (u8 *) &dev_type);
 389 
 390         if (!exec_file)
 391                 return -EFAULT;
 392 
 393         if (dev_type == DSP_UNIT) {
 394                 if (!drv_datap || !drv_datap->base_img)
 395                         return -EFAULT;
 396 
 397                 if (strlen(drv_datap->base_img) > size)
 398                         return -EINVAL;
 399 
 400                 strcpy(exec_file, drv_datap->base_img);
 401         } else if (dev_type == IVA_UNIT && iva_img) {
 402                 len = strlen(iva_img);
 403                 strncpy(exec_file, iva_img, len + 1);
 404         } else {
 405                 return -ENOENT;
 406         }
 407 
 408         return 0;
 409 }
 410 
 ...

 465         /* Get the default executable for this board... */
 466         dev_get_dev_type(hdev_obj, (u8 *) &dev_type);
 467         p_proc_object->processor_id = dev_type;
 468         status = get_exec_file(dev_node_obj, hdev_obj, sizeof(sz_exec_file),
 469                                sz_exec_file);

Re: [Suggestion] drivers/staging/tidspbridge: strcpy and strncpy, src length checking issue.

[Chen Gang]

Hello Omar Ramirez Luna:

  excuse me to bother you (maybe you are busy in these days).
  please help checking this suggestion when you have free time.

  my suggestion may be not valid (I already have at least 9 fault which I made)
  for example of my fault:
    A) net/atm:  "%pM means format this pointer as a mac address", thank Chas Williams
    B) net/tipc: "TIPC_MAX_IF_NAME is not TIPC_MAX_LINK_NAME", thank Xue Ying
    C) net/core: "not see 'if (PAGE_SIZE - len < 3)' ", find by myself
    D) MAINTAINER: "tty != serial",  thank Jiri Slaby and Joe Perches
    E) drvers/staging/telephony: "torvalds' tree is different with next tree", thank devendra.aaru
    F) drivers/staging/telephony: "we should probably fix it for older kernels", thank Dan Carpenter
    G) drivers/usb/core: "doing DMA on the stack violates the DMA rules", thank Oliver Neukum
    H) arch/blackfin/kernel: "%8s is used to take up the same space", thank Mike Frysinger and Steven Miao
    I) drivers/usb/host: "usb_hcd_giveback_urb set urb->hcpriv to NULL", thank Alan Stern

  finding and solving issues is a way (not a goal) to provide contributes to Open Source.
  so I hope: 
    When you have free time, also can provide your contributes to Open Source, too.

  thanks.


By the way:
  this week, I need work for 2 patches which relative with usb sub-system.
  if still get no reply for tidspbridge until next week.
    I should work for it, it is my duty (since I have provided 'suggestion' to it).
    "work for it" means:
       if tidspbridge is still useful
         I need construct relative environments for unit test.
         then provide relative patches.
       else (useless)
         I need delete it from Open Source.
         (since it can not pass compiling, and no response from *@ti.com, it almost means useless)
         (at least, fix the 2 compiling issues which I have suggested, can pass compiling)


  welcome any other members to giving suggestions and completions (especially from *@ti.com)


  Regards

gchen.


于 2012年12月14日 11:50, Chen Gang 写道:
> Hello Omar Ramirez Luna:
> 
>   in drivers/staging/tidspbridge/rmgr/proc.c:
> 
>     if strlen(drv_datap->base_img) == size, will pass checking (line 397)
>     the size is the full length of exec_file (line 382, line 468..469)
>     strcpy causes issue: src len is strlen(drv_datap->base_img) + '\0'. (line 400)
> 
>     strncpy seems also has issue: need use size instead of strlen(iva_img) + 1. (line 402..403)
> 
>   please help to check, thanks.
> 
> gchen.
> 
> 
>  380 static int get_exec_file(struct cfg_devnode *dev_node_obj,
>  381                                 struct dev_object *hdev_obj,
>  382                                 u32 size, char *exec_file)
>  383 {
>  384         u8 dev_type;
>  385         s32 len;
>  386         struct drv_data *drv_datap = dev_get_drvdata(bridge);
>  387 
>  388         dev_get_dev_type(hdev_obj, (u8 *) &dev_type);
>  389 
>  390         if (!exec_file)
>  391                 return -EFAULT;
>  392 
>  393         if (dev_type == DSP_UNIT) {
>  394                 if (!drv_datap || !drv_datap->base_img)
>  395                         return -EFAULT;
>  396 
>  397                 if (strlen(drv_datap->base_img) > size)
>  398                         return -EINVAL;
>  399 
>  400                 strcpy(exec_file, drv_datap->base_img);
>  401         } else if (dev_type == IVA_UNIT && iva_img) {
>  402                 len = strlen(iva_img);
>  403                 strncpy(exec_file, iva_img, len + 1);
>  404         } else {
>  405                 return -ENOENT;
>  406         }
>  407 
>  408         return 0;
>  409 }
>  410 
>  ...
> 
>  465         /* Get the default executable for this board... */
>  466         dev_get_dev_type(hdev_obj, (u8 *) &dev_type);
>  467         p_proc_object->processor_id = dev_type;
>  468         status = get_exec_file(dev_node_obj, hdev_obj, sizeof(sz_exec_file),
>  469                                sz_exec_file);
> 


-- 
Chen Gang

Asianux Corporation

Re: [Suggestion] drivers/staging/tidspbridge: strcpy and strncpy, src length checking issue.

[Chen Gang]

Hello Omar Ramirez Luna:

  excuse me to bother you (maybe you are busy in these days).
  please help checking this suggestion when you have free time.

  my suggestion may be not valid (I already have at least 9 fault which
I made)
  for example of my fault:
    A) net/atm:  "%pM means format this pointer as a mac address", thank
Chas Williams
    B) net/tipc: "TIPC_MAX_IF_NAME is not TIPC_MAX_LINK_NAME", thank Xue
Ying
    C) net/core: "not see 'if (PAGE_SIZE - len < 3)' ", find by myself
    D) MAINTAINER: "tty != serial",  thank Jiri Slaby and Joe Perches
    E) drvers/staging/telephony: "torvalds' tree is different with next
tree", thank devendra.aaru
    F) drivers/staging/telephony: "we should probably fix it for older
kernels", thank Dan Carpenter
    G) drivers/usb/core: "doing DMA on the stack violates the DMA
rules", thank Oliver Neukum
    H) arch/blackfin/kernel: "%8s is used to take up the same space",
thank Mike Frysinger and Steven Miao
    I) drivers/usb/host: "usb_hcd_giveback_urb set urb->hcpriv to NULL",
thank Alan Stern

  finding and solving issues is a way (not a goal) to provide
contributes to Open Source.
  so I hope:
    When you have free time, also can provide your contributes to Open
Source, too.

  thanks.


By the way:
  this week, I need work for 2 patches which relative with usb sub-system.
  if still get no reply for tidspbridge until next week.
    I should work for it, it is my duty (since I have provided
'suggestion' to it).
    "work for it" means:
       if tidspbridge is still useful
         I need construct relative environments for unit test.
         then provide relative patches.
       else (useless)
         I need delete it from Open Source.
         (since it can not pass compiling, and no response from
*@ti.com, it almost means useless)
         (at least, fix the 2 compiling issues which I have suggested,
can pass compiling)


  welcome any other members to giving suggestions and completions
(especially from *@ti.com)


  Regards

gchen.


于 2012年12月14日 11:50, Chen Gang 写道:
> Hello Omar Ramirez Luna:
> 
>   in drivers/staging/tidspbridge/rmgr/proc.c:
> 
>     if strlen(drv_datap->base_img) == size, will pass checking (line 397)
>     the size is the full length of exec_file (line 382, line 468..469)
>     strcpy causes issue: src len is strlen(drv_datap->base_img) + '\0'. (line 400)
> 
>     strncpy seems also has issue: need use size instead of strlen(iva_img) + 1. (line 402..403)
> 
>   please help to check, thanks.
> 
> gchen.
> 
> 
>  380 static int get_exec_file(struct cfg_devnode *dev_node_obj,
>  381                                 struct dev_object *hdev_obj,
>  382                                 u32 size, char *exec_file)
>  383 {
>  384         u8 dev_type;
>  385         s32 len;
>  386         struct drv_data *drv_datap = dev_get_drvdata(bridge);
>  387 
>  388         dev_get_dev_type(hdev_obj, (u8 *) &dev_type);
>  389 
>  390         if (!exec_file)
>  391                 return -EFAULT;
>  392 
>  393         if (dev_type == DSP_UNIT) {
>  394                 if (!drv_datap || !drv_datap->base_img)
>  395                         return -EFAULT;
>  396 
>  397                 if (strlen(drv_datap->base_img) > size)
>  398                         return -EINVAL;
>  399 
>  400                 strcpy(exec_file, drv_datap->base_img);
>  401         } else if (dev_type == IVA_UNIT && iva_img) {
>  402                 len = strlen(iva_img);
>  403                 strncpy(exec_file, iva_img, len + 1);
>  404         } else {
>  405                 return -ENOENT;
>  406         }
>  407 
>  408         return 0;
>  409 }
>  410 
>  ...
> 
>  465         /* Get the default executable for this board... */
>  466         dev_get_dev_type(hdev_obj, (u8 *) &dev_type);
>  467         p_proc_object->processor_id = dev_type;
>  468         status = get_exec_file(dev_node_obj, hdev_obj, sizeof(sz_exec_file),
>  469                                sz_exec_file);
> 


-- 
Chen Gang

Asianux Corporation

Re: [Suggestion] drivers/staging/tidspbridge: strcpy and strncpy, src length checking issue.

[Omar Ramirez Luna]

Hi Gchen,

On Mon, Dec 17, 2012 at 8:40 PM, Chen Gang <gang.chen@asianux.com> wrote:
> Hello Omar Ramirez Luna:
>
>   excuse me to bother you (maybe you are busy in these days).
>   please help checking this suggestion when you have free time.

Yes, I'm checking your suggestions, I was a little busy last week, for
the compile breaks I had a patch but I was waiting for them to show on
staging 3.8-rc1.

> By the way:
>   this week, I need work for 2 patches which relative with usb sub-system.
>   if still get no reply for tidspbridge until next week.
>     I should work for it, it is my duty (since I have provided 'suggestion' to it).
>     "work for it" means:
>        if tidspbridge is still useful
>          I need construct relative environments for unit test.
>          then provide relative patches.
>        else (useless)
>          I need delete it from Open Source.
>          (since it can not pass compiling, and no response from *@ti.com, it almost means useless)

Please DON'T assume it's useless if you don't get replies from @ti.com .

>          (at least, fix the 2 compiling issues which I have suggested, can pass compiling)

Done.

Cheers,

Omar

Re: [Suggestion] drivers/staging/tidspbridge: strcpy and strncpy, src length checking issue.

[Chen Gang]

于 2012年12月24日 22:27, Omar Ramirez Luna 写道:
> Hi Gchen,
> 
> On Mon, Dec 17, 2012 at 8:40 PM, Chen Gang <gang.chen@asianux.com> wrote:
>> Hello Omar Ramirez Luna:
>>
>>   excuse me to bother you (maybe you are busy in these days).
>>   please help checking this suggestion when you have free time.
> 
> Yes, I'm checking your suggestions, I was a little busy last week, for
> the compile breaks I had a patch but I was waiting for them to show on
> staging 3.8-rc1.
> 

  ok, I should wait for some days, at least.

  :-)

>> By the way:
>>   this week, I need work for 2 patches which relative with usb sub-system.
>>   if still get no reply for tidspbridge until next week.
>>     I should work for it, it is my duty (since I have provided 'suggestion' to it).
>>     "work for it" means:
>>        if tidspbridge is still useful
>>          I need construct relative environments for unit test.
>>          then provide relative patches.
>>        else (useless)
>>          I need delete it from Open Source.
>>          (since it can not pass compiling, and no response from *@ti.com, it almost means useless)
> 
> Please DON'T assume it's useless if you don't get replies from @ti.com .

  ok.

-- 
Chen Gang

Asianux Corporation

Re: [Suggestion] drivers/staging/tidspbridge: strcpy and strncpy, src length checking issue.

[Chen Gang]

Hi Omar Ramirez Luna:

Since you are really too busy.
  is it suitable to send a patch for it, by me ?

  no reply within a week, means it is suitable.

  :-)

gchen.


于 2012年12月14日 11:50, Chen Gang 写道:
> Hello Omar Ramirez Luna:
> 
>   in drivers/staging/tidspbridge/rmgr/proc.c:
> 
>     if strlen(drv_datap->base_img) == size, will pass checking (line 397)
>     the size is the full length of exec_file (line 382, line 468..469)
>     strcpy causes issue: src len is strlen(drv_datap->base_img) + '\0'. (line 400)
> 
>     strncpy seems also has issue: need use size instead of strlen(iva_img) + 1. (line 402..403)
> 
>   please help to check, thanks.
> 
> gchen.
> 
> 
>  380 static int get_exec_file(struct cfg_devnode *dev_node_obj,
>  381                                 struct dev_object *hdev_obj,
>  382                                 u32 size, char *exec_file)
>  383 {
>  384         u8 dev_type;
>  385         s32 len;
>  386         struct drv_data *drv_datap = dev_get_drvdata(bridge);
>  387 
>  388         dev_get_dev_type(hdev_obj, (u8 *) &dev_type);
>  389 
>  390         if (!exec_file)
>  391                 return -EFAULT;
>  392 
>  393         if (dev_type == DSP_UNIT) {
>  394                 if (!drv_datap || !drv_datap->base_img)
>  395                         return -EFAULT;
>  396 
>  397                 if (strlen(drv_datap->base_img) > size)
>  398                         return -EINVAL;
>  399 
>  400                 strcpy(exec_file, drv_datap->base_img);
>  401         } else if (dev_type == IVA_UNIT && iva_img) {
>  402                 len = strlen(iva_img);
>  403                 strncpy(exec_file, iva_img, len + 1);
>  404         } else {
>  405                 return -ENOENT;
>  406         }
>  407 
>  408         return 0;
>  409 }
>  410 
>  ...
> 
>  465         /* Get the default executable for this board... */
>  466         dev_get_dev_type(hdev_obj, (u8 *) &dev_type);
>  467         p_proc_object->processor_id = dev_type;
>  468         status = get_exec_file(dev_node_obj, hdev_obj, sizeof(sz_exec_file),
>  469                                sz_exec_file);
> 


-- 
Chen Gang

Asianux Corporation

Re: [Suggestion] drivers/staging/tidspbridge: strcpy and strncpy, src length checking issue.

[Omar Ramirez Luna]

Hi,

On Sun, Dec 30, 2012 at 9:28 PM, Chen Gang <gang.chen@asianux.com> wrote:
>   is it suitable to send a patch for it, by me ?

Thanks for your suggestions, I have created the patches and will
submit them soon, but feel free to submit patches for any other
suggestions in future.

Cheers,

Omar

Re: [Suggestion] drivers/staging/tidspbridge: strcpy and strncpy, src length checking issue.

[Omar Ramirez Luna]

Hi,

On Thu, Dec 13, 2012 at 9:50 PM, Chen Gang <gang.chen@asianux.com> wrote:
>   in drivers/staging/tidspbridge/rmgr/proc.c:
>
>     if strlen(drv_datap->base_img) == size, will pass checking (line 397)
>     the size is the full length of exec_file (line 382, line 468..469)
>     strcpy causes issue: src len is strlen(drv_datap->base_img) + '\0'. (line 400)

Good catch.

>     strncpy seems also has issue: need use size instead of strlen(iva_img) + 1. (line 402..403)

This code is not used, it is best to remove it.

Cheers,

Omar

Re: [Suggestion] drivers/staging/tidspbridge: strcpy and strncpy, src length checking issue.

[Chen Gang]

于 2013年01月05日 19:17, Omar Ramirez Luna 写道:
> Hi,
> 
> On Thu, Dec 13, 2012 at 9:50 PM, Chen Gang <gang.chen@asianux.com> wrote:
>>   in drivers/staging/tidspbridge/rmgr/proc.c:
>>
>>     if strlen(drv_datap->base_img) == size, will pass checking (line 397)
>>     the size is the full length of exec_file (line 382, line 468..469)
>>     strcpy causes issue: src len is strlen(drv_datap->base_img) + '\0'. (line 400)
> 
> Good catch.
> 
>>     strncpy seems also has issue: need use size instead of strlen(iva_img) + 1. (line 402..403)
> 
> This code is not used, it is best to remove it.
> 

  thank you.

  :-)

gchen


> Cheers,
> 
> Omar
> 
> 


-- 
Chen Gang

Asianux Corporation