Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replace 1-element array in drivers/scsi/3w-xxxx.h #206

Closed
GustavoARSilva opened this issue Sep 22, 2022 · 1 comment
Closed

Replace 1-element array in drivers/scsi/3w-xxxx.h #206

GustavoARSilva opened this issue Sep 22, 2022 · 1 comment
Assignees
@GustavoARSilva
Labels
[Idiom] fake flexible array [PATCH] Accepted A submitted patch has been accepted upstream [Refactor] 1-element array Conversion away from one-element array

Comments

@GustavoARSilva
Copy link
Collaborator

drivers/scsi/3w-xxxx.h

346 /* Structure for new chardev ioctls */                                                              
347 typedef struct TAG_TW_New_Ioctl {                                                                   
348         unsigned int data_buffer_length;                                                            
349         unsigned char padding [508];                                                                
350         TW_Command firmware_command;                                                                
351         char data_buffer[1];                                                                        
352 } TW_New_Ioctl;

Audit (at least) all these places where the flex array is being used:

diff -u -p drivers/scsi/3w-xxxx.c /tmp/nothing/3w-xxxx.c
--- drivers/scsi/3w-xxxx.c
+++ /tmp/nothing/3w-xxxx.c
@@ -933,7 +933,6 @@ static long tw_chrdev_ioctl(struct file
                        break;
                case TW_OP_AEN_LISTEN:
                        dprintk(KERN_WARNING "3w-xxxx: tw_chrdev_ioctl(): caught TW_AEN_LISTEN.\n");
-                       memset(tw_ioctl->data_buffer, 0, data_buffer_length);

                        spin_lock_irqsave(tw_dev->host->host_lock, flags);
                        if (tw_dev->aen_head == tw_dev->aen_tail) {
@@ -947,7 +946,6 @@ static long tw_chrdev_ioctl(struct file
                                }
                        }
                        spin_unlock_irqrestore(tw_dev->host->host_lock, flags);
-                       memcpy(tw_ioctl->data_buffer, &tw_aen_code, sizeof(tw_aen_code));
                        break;
                case TW_CMD_PACKET_WITH_DATA:
                        dprintk(KERN_WARNING "3w-xxxx: tw_chrdev_ioctl(): caught TW_CMD_PACKET_WITH_DATA.\n");
@GustavoARSilva GustavoARSilva self-assigned this Sep 22, 2022
@GustavoARSilva GustavoARSilva added [PATCH] Exists A patch exists to address the issue [Refactor] 1-element array Conversion away from one-element array [Idiom] fake flexible array labels Sep 22, 2022
@GustavoARSilva
Copy link
Collaborator Author

I just sent a patch for this:
https://lore.kernel.org/linux-hardening/YyyyvB30jnjRaw%2FF@work/

intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Sep 22, 2022
@GustavoARSilva @intel-lab-lkp
scsi: 3w-xxxx: Replace one-element array with flexible-array member
aae6954 
One-element arrays are deprecated, and we are replacing them with flexible
array members instead. So, replace one-element array with flexible-array
member in struct TAG_TW_New_Ioctl and refactor the rest of the code,
accordingly.

Notice that, in multiple places, the subtraction of 1 from
sizeof(TW_New_Ioctl) is removed, as this operation is now implicit
after the flex-array transformation.

Link: KSPP#79
Link: KSPP#206
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Sep 25, 2022
@GustavoARSilva @martinkpetersen
scsi: 3w-xxxx: Replace one-element array with flexible-array member
0fb9125 
One-element arrays are deprecated, and we are replacing them with flexible
array members instead. So, replace one-element array with flexible-array
member in struct TAG_TW_New_Ioctl and refactor the rest of the code,
accordingly.

Notice that, in multiple places, the subtraction of 1 from
sizeof(TW_New_Ioctl) is removed, as this operation is now implicit
after the flex-array transformation.

Link: KSPP#79
Link: KSPP#206
Link: https://lore.kernel.org/r/YyyyvB30jnjRaw/F@work
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
@GustavoARSilva GustavoARSilva added [PATCH] Accepted A submitted patch has been accepted upstream and removed [PATCH] Exists A patch exists to address the issue labels Oct 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@GustavoARSilva GustavoARSilva

Labels
[Idiom] fake flexible array [PATCH] Accepted A submitted patch has been accepted upstream [Refactor] 1-element array Conversion away from one-element array
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@GustavoARSilva