| Message ID | CADT32eKtWMF+8+jf=UWh8hsuNGA6GoTqP-+s0H8E3U=6uhX61A@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Shirish, I've tested the patch, and as far as I can see it works perfectly. The extra zero bytes are no longer in the request and the server now accepts them. I've tested on a number of directories, with a number of different ACEs, adding and deleting, and could not find an issue so far. I've not tested the smb2 patches for setting acl's yet, as I do not have an environment ready to go for recompiling the cifs kernel module. The server we are using is a Hitachi HNAS: https://www.hds.com/en-us/products-solutions/storage/network-attached-storage-platform.html Thanks, Paul On Wed, Jul 5, 2017 at 7:08 AM, Shirish Pargaonkar <shirishpargaonkar@gmail.com> wrote: > Can you try this patch, I did preliminary testing... > > > > On Tue, Jul 4, 2017 at 4:17 PM, Shirish Pargaonkar > <shirishpargaonkar@gmail.com> wrote: >> Hi Paul, >> >> So something like this works with smb 2.1 with the patchset I posted >> on the mailing list >> to make set acl work over smb 2 onwards... >> >> $ sudo ls -l /mnt/ >> total 1 >> -rwx------ 1 root root 32 Jan 26 2015 file1.txt >> -rwx------ 1 root root 0 Nov 30 2014 file2.txt >> drwx------ 2 root root 0 Nov 30 2014 folder1 >> drwx------ 2 root root 0 Dec 22 2014 folder2 >> >> $ sudo setcifsacl -S >> "ACL:S-1-5-21-609720889-20222076-3464071719-1001:ALLOWED/0x0/FULL,ACL:S-1-5-21-609720889-20222076-3464071719-513:ALLOWED/0x0/0x1f01b9,ACL:\Everyone:ALLOWED/0x0/0x1f01b9" >> /mnt/file1.txt >> >> $ sudo ls -l /mnt/ >> total 1 >> -rwxr-xr-x 1 root root 32 Jan 26 2015 file1.txt >> -rwx------ 1 root root 0 Nov 30 2014 file2.txt >> drwx------ 2 root root 0 Nov 30 2014 folder1 >> drwx------ 2 root root 0 Dec 22 2014 folder2 >> >> I then changed it back to 700 using smb1 set cifs acl and it worked too. >> >> $ sudo setcifsacl -S >> "ACL:S-1-5-21-609720889-20222076-3464071719-1001:ALLOWED/0x0/FULL,ACL:S-1-5-21-609720889-20222076-3464071719-513:ALLOWED/0x0/0x120088,ACL:\Everyone:ALLOWED/0x0/0x120088" >> /mnt/file1.txt >> >> $ sudo ls -l /mnt/ >> total 1 >> -rwx------ 1 root root 32 Jan 26 2015 file1.txt >> -rwx------ 1 root root 0 Nov 30 2014 file2.txt >> drwx------ 2 root root 0 Nov 30 2014 folder1 >> drwx------ 2 root root 0 Dec 22 2014 folder2 >> >> What type of Windows server is this? These requests succeed but I do >> see extra bytes >> at the end of the security descriptor in both the requests, trying to >> figure out why is it allocated in the first place... >> >> >> On Tue, Jun 27, 2017 at 5:43 AM, Paul van Schayck <polleke@gmail.com> wrote: >>> Hi Shirish, >>> >>> Thank you for your answer. >>> >>> Attached you can find three pcap files. >>> >>> 1. linux-fail.pcap: Using the setcifsacl 6.7 version. >>> 2. linux-succes.pcap: Using the modified setcifsacl with the patch >>> above (note, I had to change to len + 4 in trim_request()) >>> 3. windows-succes-trimmed.pcap: Using the dialog in Windows. Note that >>> this is over smbv2. The file was trimmed down, because windows was >>> noisy. >>> >>> The command I used was >>> >>> setcifsacl -S "ACL:S-1-5-21-1572361299-1184395705-1606240830-247332:ALLOWED/OI|CI/CHANGE,ACL:S-1-5-21-1572361299-1184395705-1606240830-544559:ALLOWED/I/FULL,ACL:S-1-3-0:ALLOWED/OI|CI|IO|I/FULL,ACL:S-1-5-21-1572361299-1184395705-1606240830-512:ALLOWED/OI|CI|I/FULL,ACL:S-1-5-21-1572361299-1184395705-1606240830-549266:ALLOWED/OI|CI|I/FULL,ACL:S-1-5-21-1572361299-1184395705-1606240830-544559:ALLOWED/OI|CI|IO|I/FULL,ACL:S-1-5-18:ALLOWED/OI|CI|I/FULL" >>> cifs-test/ >>> >>> In Windows I used the dialog to set the same ACEs. Between the >>> linux-success and linux-fail file you can see the difference in the >>> number of zero bytes after the DACL. >>> >>> If this test case is still a bit noisy for you, I can try to build a >>> more simple ACE list. But I have to setup a new share for this. >>> >>> Thanks, >>> >>> Paul van Schayck >>> >>> On Tue, Jun 27, 2017 at 6:40 AM, Shirish Pargaonkar >>> <shirishpargaonkar@gmail.com> wrote: >>>> if you can provide tcpdumpas well as the command you are using of/for >>>> both Windows and Linux way, that would be helpful >>>> >>>> On Mon, Jun 26, 2017 at 8:04 AM, Paul van Schayck via samba-technical >>>> <samba-technical@lists.samba.org> wrote: >>>>> Dear samba-technical, >>>>> >>>>> If this list is inappropriate for this question then please redirect >>>>> me to a better one. >>>>> >>>>> I'm working with a HDS HNAS (Hitachi NAS) which has its own >>>>> implementation of the CIFS protocol. Mounting, and interactions all go >>>>> fine using linux-cifs and related tools. >>>>> >>>>> However, when sending a `setcifsacl -a ACE /dir` the response is >>>>> "main: setxattr error: Input/output error". Upon further digging using >>>>> tcpdump/wireshark it became clear that the HNAS is responding with >>>>> >>>>> NT Status: STATUS_INVALID_SECURITY_DESCR (0xc0000079) >>>>> >>>>> Setting the same ACE using a Windows client did work. So further >>>>> examining of the request being sent showed that the only difference >>>>> between the Windows client and setcifsacl was the addition of a lot of >>>>> padding zero bytes at the end of the security descriptor. >>>>> >>>>> Examining the code of setcifsacl.c and the request more it seems like >>>>> the request was not properly trimmed down to the number of aces being >>>>> sent. More buffer was allocated that the number of aces being sent. So >>>>> I made the attached patch to trim down the request. This fixes some of >>>>> the problems with setcifsacl, but for example breaks deleting aces. I >>>>> also think it's most likely an ugly fix, and the problem needs to be >>>>> solved elsewhere most likely. >>>>> >>>>> So my question is if someone can point me at the correct way to fix >>>>> this in setcifsacl, or help me fix it properly. If necessary I can >>>>> provide tcpdump's of Windows and Linux clients performing the request. >>>>> >>>>> Thanks, >>>>> >>>>> Paul van Schayck >>>>> >>>>> diff --git a/setcifsacl.c b/setcifsacl.c >>>>> index 7eeeaa6..4ada3c8 100644 >>>>> --- a/setcifsacl.c >>>>> +++ b/setcifsacl.c >>>>> @@ -761,6 +761,20 @@ setacl_action(struct cifs_ntsd *pntsd, struct >>>>> cifs_ntsd **npntsd, >>>>> return rc; >>>>> } >>>>> >>>>> +ssize_t >>>>> +trim_request(ssize_t bufsize, struct cifs_ntsd *npntsd) { >>>>> + int i, len = 0; >>>>> + char *a = (char *) npntsd; >>>>> + >>>>> + for( i = 0; i < bufsize; i++) { >>>>> + if ( a[i] != 0 ) >>>>> + len = i; >>>>> + } >>>>> + >>>>> + return (ssize_t) len + 2; >>>>> +} >>>>> + >>>>> + >>>>> static void >>>>> setcifsacl_usage(const char *prog) >>>>> { >>>>> @@ -902,7 +916,7 @@ cifsacl: >>>>> if (rc) >>>>> goto setcifsacl_action_ret; >>>>> >>>>> - attrlen = setxattr(filename, ATTRNAME, ntsdptr, bufsize, 0); >>>>> + attrlen = setxattr(filename, ATTRNAME, ntsdptr, >>>>> trim_request(bufsize, ntsdptr), 0); >>>>> if (attrlen == -1) { >>>>> printf("%s: setxattr error: %s\n", __func__, strerror(errno)); >>>>> goto setcifsacl_facenum_ret; >>>>> -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Somehow I missed this response. Out till Friday, will submit this patch once I am back, next week. Thanks for testing the patch. Regards, Shirish On Tue, Jul 11, 2017 at 7:13 AM, Paul van Schayck <polleke@gmail.com> wrote: > Hi Shirish, > > I've tested the patch, and as far as I can see it works perfectly. The > extra zero bytes are no longer in the request and the server now > accepts them. I've tested on a number of directories, with a number of > different ACEs, adding and deleting, and could not find an issue so > far. > > I've not tested the smb2 patches for setting acl's yet, as I do not > have an environment ready to go for recompiling the cifs kernel > module. > > The server we are using is a Hitachi HNAS: > https://www.hds.com/en-us/products-solutions/storage/network-attached-storage-platform.html > > Thanks, > > Paul > > On Wed, Jul 5, 2017 at 7:08 AM, Shirish Pargaonkar > <shirishpargaonkar@gmail.com> wrote: >> Can you try this patch, I did preliminary testing... >> >> >> >> On Tue, Jul 4, 2017 at 4:17 PM, Shirish Pargaonkar >> <shirishpargaonkar@gmail.com> wrote: >>> Hi Paul, >>> >>> So something like this works with smb 2.1 with the patchset I posted >>> on the mailing list >>> to make set acl work over smb 2 onwards... >>> >>> $ sudo ls -l /mnt/ >>> total 1 >>> -rwx------ 1 root root 32 Jan 26 2015 file1.txt >>> -rwx------ 1 root root 0 Nov 30 2014 file2.txt >>> drwx------ 2 root root 0 Nov 30 2014 folder1 >>> drwx------ 2 root root 0 Dec 22 2014 folder2 >>> >>> $ sudo setcifsacl -S >>> "ACL:S-1-5-21-609720889-20222076-3464071719-1001:ALLOWED/0x0/FULL,ACL:S-1-5-21-609720889-20222076-3464071719-513:ALLOWED/0x0/0x1f01b9,ACL:\Everyone:ALLOWED/0x0/0x1f01b9" >>> /mnt/file1.txt >>> >>> $ sudo ls -l /mnt/ >>> total 1 >>> -rwxr-xr-x 1 root root 32 Jan 26 2015 file1.txt >>> -rwx------ 1 root root 0 Nov 30 2014 file2.txt >>> drwx------ 2 root root 0 Nov 30 2014 folder1 >>> drwx------ 2 root root 0 Dec 22 2014 folder2 >>> >>> I then changed it back to 700 using smb1 set cifs acl and it worked too. >>> >>> $ sudo setcifsacl -S >>> "ACL:S-1-5-21-609720889-20222076-3464071719-1001:ALLOWED/0x0/FULL,ACL:S-1-5-21-609720889-20222076-3464071719-513:ALLOWED/0x0/0x120088,ACL:\Everyone:ALLOWED/0x0/0x120088" >>> /mnt/file1.txt >>> >>> $ sudo ls -l /mnt/ >>> total 1 >>> -rwx------ 1 root root 32 Jan 26 2015 file1.txt >>> -rwx------ 1 root root 0 Nov 30 2014 file2.txt >>> drwx------ 2 root root 0 Nov 30 2014 folder1 >>> drwx------ 2 root root 0 Dec 22 2014 folder2 >>> >>> What type of Windows server is this? These requests succeed but I do >>> see extra bytes >>> at the end of the security descriptor in both the requests, trying to >>> figure out why is it allocated in the first place... >>> >>> >>> On Tue, Jun 27, 2017 at 5:43 AM, Paul van Schayck <polleke@gmail.com> wrote: >>>> Hi Shirish, >>>> >>>> Thank you for your answer. >>>> >>>> Attached you can find three pcap files. >>>> >>>> 1. linux-fail.pcap: Using the setcifsacl 6.7 version. >>>> 2. linux-succes.pcap: Using the modified setcifsacl with the patch >>>> above (note, I had to change to len + 4 in trim_request()) >>>> 3. windows-succes-trimmed.pcap: Using the dialog in Windows. Note that >>>> this is over smbv2. The file was trimmed down, because windows was >>>> noisy. >>>> >>>> The command I used was >>>> >>>> setcifsacl -S "ACL:S-1-5-21-1572361299-1184395705-1606240830-247332:ALLOWED/OI|CI/CHANGE,ACL:S-1-5-21-1572361299-1184395705-1606240830-544559:ALLOWED/I/FULL,ACL:S-1-3-0:ALLOWED/OI|CI|IO|I/FULL,ACL:S-1-5-21-1572361299-1184395705-1606240830-512:ALLOWED/OI|CI|I/FULL,ACL:S-1-5-21-1572361299-1184395705-1606240830-549266:ALLOWED/OI|CI|I/FULL,ACL:S-1-5-21-1572361299-1184395705-1606240830-544559:ALLOWED/OI|CI|IO|I/FULL,ACL:S-1-5-18:ALLOWED/OI|CI|I/FULL" >>>> cifs-test/ >>>> >>>> In Windows I used the dialog to set the same ACEs. Between the >>>> linux-success and linux-fail file you can see the difference in the >>>> number of zero bytes after the DACL. >>>> >>>> If this test case is still a bit noisy for you, I can try to build a >>>> more simple ACE list. But I have to setup a new share for this. >>>> >>>> Thanks, >>>> >>>> Paul van Schayck >>>> >>>> On Tue, Jun 27, 2017 at 6:40 AM, Shirish Pargaonkar >>>> <shirishpargaonkar@gmail.com> wrote: >>>>> if you can provide tcpdumpas well as the command you are using of/for >>>>> both Windows and Linux way, that would be helpful >>>>> >>>>> On Mon, Jun 26, 2017 at 8:04 AM, Paul van Schayck via samba-technical >>>>> <samba-technical@lists.samba.org> wrote: >>>>>> Dear samba-technical, >>>>>> >>>>>> If this list is inappropriate for this question then please redirect >>>>>> me to a better one. >>>>>> >>>>>> I'm working with a HDS HNAS (Hitachi NAS) which has its own >>>>>> implementation of the CIFS protocol. Mounting, and interactions all go >>>>>> fine using linux-cifs and related tools. >>>>>> >>>>>> However, when sending a `setcifsacl -a ACE /dir` the response is >>>>>> "main: setxattr error: Input/output error". Upon further digging using >>>>>> tcpdump/wireshark it became clear that the HNAS is responding with >>>>>> >>>>>> NT Status: STATUS_INVALID_SECURITY_DESCR (0xc0000079) >>>>>> >>>>>> Setting the same ACE using a Windows client did work. So further >>>>>> examining of the request being sent showed that the only difference >>>>>> between the Windows client and setcifsacl was the addition of a lot of >>>>>> padding zero bytes at the end of the security descriptor. >>>>>> >>>>>> Examining the code of setcifsacl.c and the request more it seems like >>>>>> the request was not properly trimmed down to the number of aces being >>>>>> sent. More buffer was allocated that the number of aces being sent. So >>>>>> I made the attached patch to trim down the request. This fixes some of >>>>>> the problems with setcifsacl, but for example breaks deleting aces. I >>>>>> also think it's most likely an ugly fix, and the problem needs to be >>>>>> solved elsewhere most likely. >>>>>> >>>>>> So my question is if someone can point me at the correct way to fix >>>>>> this in setcifsacl, or help me fix it properly. If necessary I can >>>>>> provide tcpdump's of Windows and Linux clients performing the request. >>>>>> >>>>>> Thanks, >>>>>> >>>>>> Paul van Schayck >>>>>> >>>>>> diff --git a/setcifsacl.c b/setcifsacl.c >>>>>> index 7eeeaa6..4ada3c8 100644 >>>>>> --- a/setcifsacl.c >>>>>> +++ b/setcifsacl.c >>>>>> @@ -761,6 +761,20 @@ setacl_action(struct cifs_ntsd *pntsd, struct >>>>>> cifs_ntsd **npntsd, >>>>>> return rc; >>>>>> } >>>>>> >>>>>> +ssize_t >>>>>> +trim_request(ssize_t bufsize, struct cifs_ntsd *npntsd) { >>>>>> + int i, len = 0; >>>>>> + char *a = (char *) npntsd; >>>>>> + >>>>>> + for( i = 0; i < bufsize; i++) { >>>>>> + if ( a[i] != 0 ) >>>>>> + len = i; >>>>>> + } >>>>>> + >>>>>> + return (ssize_t) len + 2; >>>>>> +} >>>>>> + >>>>>> + >>>>>> static void >>>>>> setcifsacl_usage(const char *prog) >>>>>> { >>>>>> @@ -902,7 +916,7 @@ cifsacl: >>>>>> if (rc) >>>>>> goto setcifsacl_action_ret; >>>>>> >>>>>> - attrlen = setxattr(filename, ATTRNAME, ntsdptr, bufsize, 0); >>>>>> + attrlen = setxattr(filename, ATTRNAME, ntsdptr, >>>>>> trim_request(bufsize, ntsdptr), 0); >>>>>> if (attrlen == -1) { >>>>>> printf("%s: setxattr error: %s\n", __func__, strerror(errno)); >>>>>> goto setcifsacl_facenum_ret; >>>>>> -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Shirish, Thanks, that would be great if you would be able to submit the setcifsacl patch. We're using it with great success in our environment. Cheers, Paul On Sun, Aug 13, 2017 at 3:33 AM, Shirish Pargaonkar <shirishpargaonkar@gmail.com> wrote: > Somehow I missed this response. Out till Friday, will submit this > patch once I am back, next week. Thanks for testing the patch. > > Regards, > > Shirish > > On Tue, Jul 11, 2017 at 7:13 AM, Paul van Schayck <polleke@gmail.com> wrote: >> Hi Shirish, >> >> I've tested the patch, and as far as I can see it works perfectly. The >> extra zero bytes are no longer in the request and the server now >> accepts them. I've tested on a number of directories, with a number of >> different ACEs, adding and deleting, and could not find an issue so >> far. >> >> I've not tested the smb2 patches for setting acl's yet, as I do not >> have an environment ready to go for recompiling the cifs kernel >> module. >> >> The server we are using is a Hitachi HNAS: >> https://www.hds.com/en-us/products-solutions/storage/network-attached-storage-platform.html >> >> Thanks, >> >> Paul >> >> On Wed, Jul 5, 2017 at 7:08 AM, Shirish Pargaonkar >> <shirishpargaonkar@gmail.com> wrote: >>> Can you try this patch, I did preliminary testing... >>> >>> >>> >>> On Tue, Jul 4, 2017 at 4:17 PM, Shirish Pargaonkar >>> <shirishpargaonkar@gmail.com> wrote: >>>> Hi Paul, >>>> >>>> So something like this works with smb 2.1 with the patchset I posted >>>> on the mailing list >>>> to make set acl work over smb 2 onwards... >>>> >>>> $ sudo ls -l /mnt/ >>>> total 1 >>>> -rwx------ 1 root root 32 Jan 26 2015 file1.txt >>>> -rwx------ 1 root root 0 Nov 30 2014 file2.txt >>>> drwx------ 2 root root 0 Nov 30 2014 folder1 >>>> drwx------ 2 root root 0 Dec 22 2014 folder2 >>>> >>>> $ sudo setcifsacl -S >>>> "ACL:S-1-5-21-609720889-20222076-3464071719-1001:ALLOWED/0x0/FULL,ACL:S-1-5-21-609720889-20222076-3464071719-513:ALLOWED/0x0/0x1f01b9,ACL:\Everyone:ALLOWED/0x0/0x1f01b9" >>>> /mnt/file1.txt >>>> >>>> $ sudo ls -l /mnt/ >>>> total 1 >>>> -rwxr-xr-x 1 root root 32 Jan 26 2015 file1.txt >>>> -rwx------ 1 root root 0 Nov 30 2014 file2.txt >>>> drwx------ 2 root root 0 Nov 30 2014 folder1 >>>> drwx------ 2 root root 0 Dec 22 2014 folder2 >>>> >>>> I then changed it back to 700 using smb1 set cifs acl and it worked too. >>>> >>>> $ sudo setcifsacl -S >>>> "ACL:S-1-5-21-609720889-20222076-3464071719-1001:ALLOWED/0x0/FULL,ACL:S-1-5-21-609720889-20222076-3464071719-513:ALLOWED/0x0/0x120088,ACL:\Everyone:ALLOWED/0x0/0x120088" >>>> /mnt/file1.txt >>>> >>>> $ sudo ls -l /mnt/ >>>> total 1 >>>> -rwx------ 1 root root 32 Jan 26 2015 file1.txt >>>> -rwx------ 1 root root 0 Nov 30 2014 file2.txt >>>> drwx------ 2 root root 0 Nov 30 2014 folder1 >>>> drwx------ 2 root root 0 Dec 22 2014 folder2 >>>> >>>> What type of Windows server is this? These requests succeed but I do >>>> see extra bytes >>>> at the end of the security descriptor in both the requests, trying to >>>> figure out why is it allocated in the first place... >>>> >>>> >>>> On Tue, Jun 27, 2017 at 5:43 AM, Paul van Schayck <polleke@gmail.com> wrote: >>>>> Hi Shirish, >>>>> >>>>> Thank you for your answer. >>>>> >>>>> Attached you can find three pcap files. >>>>> >>>>> 1. linux-fail.pcap: Using the setcifsacl 6.7 version. >>>>> 2. linux-succes.pcap: Using the modified setcifsacl with the patch >>>>> above (note, I had to change to len + 4 in trim_request()) >>>>> 3. windows-succes-trimmed.pcap: Using the dialog in Windows. Note that >>>>> this is over smbv2. The file was trimmed down, because windows was >>>>> noisy. >>>>> >>>>> The command I used was >>>>> >>>>> setcifsacl -S "ACL:S-1-5-21-1572361299-1184395705-1606240830-247332:ALLOWED/OI|CI/CHANGE,ACL:S-1-5-21-1572361299-1184395705-1606240830-544559:ALLOWED/I/FULL,ACL:S-1-3-0:ALLOWED/OI|CI|IO|I/FULL,ACL:S-1-5-21-1572361299-1184395705-1606240830-512:ALLOWED/OI|CI|I/FULL,ACL:S-1-5-21-1572361299-1184395705-1606240830-549266:ALLOWED/OI|CI|I/FULL,ACL:S-1-5-21-1572361299-1184395705-1606240830-544559:ALLOWED/OI|CI|IO|I/FULL,ACL:S-1-5-18:ALLOWED/OI|CI|I/FULL" >>>>> cifs-test/ >>>>> >>>>> In Windows I used the dialog to set the same ACEs. Between the >>>>> linux-success and linux-fail file you can see the difference in the >>>>> number of zero bytes after the DACL. >>>>> >>>>> If this test case is still a bit noisy for you, I can try to build a >>>>> more simple ACE list. But I have to setup a new share for this. >>>>> >>>>> Thanks, >>>>> >>>>> Paul van Schayck >>>>> >>>>> On Tue, Jun 27, 2017 at 6:40 AM, Shirish Pargaonkar >>>>> <shirishpargaonkar@gmail.com> wrote: >>>>>> if you can provide tcpdumpas well as the command you are using of/for >>>>>> both Windows and Linux way, that would be helpful >>>>>> >>>>>> On Mon, Jun 26, 2017 at 8:04 AM, Paul van Schayck via samba-technical >>>>>> <samba-technical@lists.samba.org> wrote: >>>>>>> Dear samba-technical, >>>>>>> >>>>>>> If this list is inappropriate for this question then please redirect >>>>>>> me to a better one. >>>>>>> >>>>>>> I'm working with a HDS HNAS (Hitachi NAS) which has its own >>>>>>> implementation of the CIFS protocol. Mounting, and interactions all go >>>>>>> fine using linux-cifs and related tools. >>>>>>> >>>>>>> However, when sending a `setcifsacl -a ACE /dir` the response is >>>>>>> "main: setxattr error: Input/output error". Upon further digging using >>>>>>> tcpdump/wireshark it became clear that the HNAS is responding with >>>>>>> >>>>>>> NT Status: STATUS_INVALID_SECURITY_DESCR (0xc0000079) >>>>>>> >>>>>>> Setting the same ACE using a Windows client did work. So further >>>>>>> examining of the request being sent showed that the only difference >>>>>>> between the Windows client and setcifsacl was the addition of a lot of >>>>>>> padding zero bytes at the end of the security descriptor. >>>>>>> >>>>>>> Examining the code of setcifsacl.c and the request more it seems like >>>>>>> the request was not properly trimmed down to the number of aces being >>>>>>> sent. More buffer was allocated that the number of aces being sent. So >>>>>>> I made the attached patch to trim down the request. This fixes some of >>>>>>> the problems with setcifsacl, but for example breaks deleting aces. I >>>>>>> also think it's most likely an ugly fix, and the problem needs to be >>>>>>> solved elsewhere most likely. >>>>>>> >>>>>>> So my question is if someone can point me at the correct way to fix >>>>>>> this in setcifsacl, or help me fix it properly. If necessary I can >>>>>>> provide tcpdump's of Windows and Linux clients performing the request. >>>>>>> >>>>>>> Thanks, >>>>>>> >>>>>>> Paul van Schayck >>>>>>> >>>>>>> diff --git a/setcifsacl.c b/setcifsacl.c >>>>>>> index 7eeeaa6..4ada3c8 100644 >>>>>>> --- a/setcifsacl.c >>>>>>> +++ b/setcifsacl.c >>>>>>> @@ -761,6 +761,20 @@ setacl_action(struct cifs_ntsd *pntsd, struct >>>>>>> cifs_ntsd **npntsd, >>>>>>> return rc; >>>>>>> } >>>>>>> >>>>>>> +ssize_t >>>>>>> +trim_request(ssize_t bufsize, struct cifs_ntsd *npntsd) { >>>>>>> + int i, len = 0; >>>>>>> + char *a = (char *) npntsd; >>>>>>> + >>>>>>> + for( i = 0; i < bufsize; i++) { >>>>>>> + if ( a[i] != 0 ) >>>>>>> + len = i; >>>>>>> + } >>>>>>> + >>>>>>> + return (ssize_t) len + 2; >>>>>>> +} >>>>>>> + >>>>>>> + >>>>>>> static void >>>>>>> setcifsacl_usage(const char *prog) >>>>>>> { >>>>>>> @@ -902,7 +916,7 @@ cifsacl: >>>>>>> if (rc) >>>>>>> goto setcifsacl_action_ret; >>>>>>> >>>>>>> - attrlen = setxattr(filename, ATTRNAME, ntsdptr, bufsize, 0); >>>>>>> + attrlen = setxattr(filename, ATTRNAME, ntsdptr, >>>>>>> trim_request(bufsize, ntsdptr), 0); >>>>>>> if (attrlen == -1) { >>>>>>> printf("%s: setxattr error: %s\n", __func__, strerror(errno)); >>>>>>> goto setcifsacl_facenum_ret; >>>>>>> -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/setcifsacl.c b/setcifsacl.c index 7eeeaa6..0d4b15f 100644 --- a/setcifsacl.c +++ b/setcifsacl.c @@ -50,24 +50,33 @@ enum setcifsacl_actions { static void *plugin_handle; static bool plugin_loaded; -static void +static int copy_cifs_sid(struct cifs_sid *dst, const struct cifs_sid *src) { - int i; + int i, size = 0; dst->revision = src->revision; + size += sizeof(uint8_t); + dst->num_subauth = src->num_subauth; + size += sizeof(uint8_t); + for (i = 0; i < NUM_AUTHS; i++) dst->authority[i] = src->authority[i]; + size += (sizeof(uint8_t) * NUM_AUTHS); + for (i = 0; i < src->num_subauth; i++) dst->sub_auth[i] = src->sub_auth[i]; + size += (sizeof(uint32_t) * src->num_subauth); + + return size; } static void copy_sec_desc(const struct cifs_ntsd *pntsd, struct cifs_ntsd *pnntsd, - int numaces, int acessize) + int numaces, int acessize, ssize_t *bufsize) { - int osidsoffset, gsidsoffset, dacloffset; + int size, osidsoffset, gsidsoffset, dacloffset; struct cifs_sid *owner_sid_ptr, *group_sid_ptr; struct cifs_sid *nowner_sid_ptr, *ngroup_sid_ptr; struct cifs_ctrl_acl *dacl_ptr, *ndacl_ptr; @@ -77,28 +86,34 @@ copy_sec_desc(const struct cifs_ntsd *pntsd, struct cifs_ntsd *pnntsd, gsidsoffset = le32toh(pntsd->gsidoffset); dacloffset = le32toh(pntsd->dacloffset); + size = sizeof(struct cifs_ntsd); pnntsd->revision = pntsd->revision; pnntsd->type = pntsd->type; pnntsd->osidoffset = pntsd->osidoffset; pnntsd->gsidoffset = pntsd->gsidoffset; pnntsd->dacloffset = pntsd->dacloffset; + *bufsize = *bufsize + size; dacl_ptr = (struct cifs_ctrl_acl *)((char *)pntsd + dacloffset); ndacl_ptr = (struct cifs_ctrl_acl *)((char *)pnntsd + dacloffset); + size = acessize + sizeof(struct cifs_ctrl_acl); ndacl_ptr->revision = dacl_ptr->revision; - ndacl_ptr->size = htole16(acessize + sizeof(struct cifs_ctrl_acl)); + ndacl_ptr->size = htole16(size); ndacl_ptr->num_aces = htole32(numaces); + *bufsize = *bufsize + size; /* copy owner sid */ owner_sid_ptr = (struct cifs_sid *)((char *)pntsd + osidsoffset); nowner_sid_ptr = (struct cifs_sid *)((char *)pnntsd + osidsoffset); - copy_cifs_sid(nowner_sid_ptr, owner_sid_ptr); + size = copy_cifs_sid(nowner_sid_ptr, owner_sid_ptr); + *bufsize = *bufsize + size; /* copy group sid */ group_sid_ptr = (struct cifs_sid *)((char *)pntsd + gsidsoffset); ngroup_sid_ptr = (struct cifs_sid *)((char *)pnntsd + gsidsoffset); - copy_cifs_sid(ngroup_sid_ptr, group_sid_ptr); + size = copy_cifs_sid(ngroup_sid_ptr, group_sid_ptr); + *bufsize = *bufsize + size; return; } @@ -156,10 +171,10 @@ compare_aces(struct cifs_ace *sace, struct cifs_ace *dace, int compflags) } static int -get_sec_desc_size(struct cifs_ntsd *pntsd, struct cifs_ntsd **npntsd, - int aces, ssize_t *bufsize, size_t *acesoffset) +alloc_sec_desc(struct cifs_ntsd *pntsd, struct cifs_ntsd **npntsd, + int aces, size_t *acesoffset) { - unsigned int size, acessize, dacloffset; + unsigned int size, acessize, bufsize, dacloffset; size = sizeof(struct cifs_ntsd) + 2 * sizeof(struct cifs_sid) + @@ -169,9 +184,9 @@ get_sec_desc_size(struct cifs_ntsd *pntsd, struct cifs_ntsd **npntsd, *acesoffset = dacloffset + sizeof(struct cifs_ctrl_acl); acessize = aces * sizeof(struct cifs_ace); - *bufsize = size + acessize; + bufsize = size + acessize; - *npntsd = malloc(*bufsize); + *npntsd = malloc(bufsize); if (!*npntsd) { printf("%s: Memory allocation failure", __func__); return errno; @@ -188,7 +203,7 @@ ace_set(struct cifs_ntsd *pntsd, struct cifs_ntsd **npntsd, ssize_t *bufsize, size_t acesoffset; char *acesptr; - rc = get_sec_desc_size(pntsd, npntsd, numcaces, bufsize, &acesoffset); + rc = alloc_sec_desc(pntsd, npntsd, numcaces, &acesoffset); if (rc) return rc; @@ -198,9 +213,8 @@ ace_set(struct cifs_ntsd *pntsd, struct cifs_ntsd **npntsd, ssize_t *bufsize, acessize += size; acesptr += size; } - copy_sec_desc(pntsd, *npntsd, numcaces, acessize); - acesptr = (char *)*npntsd + acesoffset; + copy_sec_desc(pntsd, *npntsd, numcaces, acessize, bufsize); return 0; } @@ -215,7 +229,7 @@ ace_add(struct cifs_ntsd *pntsd, struct cifs_ntsd **npntsd, ssize_t *bufsize, char *acesptr; numaces = numfaces + numcaces; - rc = get_sec_desc_size(pntsd, npntsd, numaces, bufsize, &acesoffset); + rc = alloc_sec_desc(pntsd, npntsd, numaces, &acesoffset); if (rc) return rc; @@ -230,7 +244,8 @@ ace_add(struct cifs_ntsd *pntsd, struct cifs_ntsd **npntsd, ssize_t *bufsize, acesptr += size; acessize += size; } - copy_sec_desc(pntsd, *npntsd, numaces, acessize); + + copy_sec_desc(pntsd, *npntsd, numaces, acessize, bufsize); return 0; } @@ -249,7 +264,7 @@ ace_modify(struct cifs_ntsd *pntsd, struct cifs_ntsd **npntsd, ssize_t *bufsize, return -1; } - rc = get_sec_desc_size(pntsd, npntsd, numfaces, bufsize, &acesoffset); + rc = alloc_sec_desc(pntsd, npntsd, numfaces, &acesoffset); if (rc) return rc; @@ -270,7 +285,7 @@ ace_modify(struct cifs_ntsd *pntsd, struct cifs_ntsd **npntsd, ssize_t *bufsize, acessize += size; } - copy_sec_desc(pntsd, *npntsd, numfaces, acessize); + copy_sec_desc(pntsd, *npntsd, numfaces, acessize, bufsize); return 0; } @@ -294,7 +309,7 @@ ace_delete(struct cifs_ntsd *pntsd, struct cifs_ntsd **npntsd, ssize_t *bufsize, return -1; } - rc = get_sec_desc_size(pntsd, npntsd, numfaces, bufsize, &acesoffset); + rc = alloc_sec_desc(pntsd, npntsd, numfaces, &acesoffset); if (rc) return rc; @@ -317,7 +332,8 @@ ace_delete(struct cifs_ntsd *pntsd, struct cifs_ntsd **npntsd, ssize_t *bufsize, printf("%s: Nothing to delete\n", __func__); return 1; } - copy_sec_desc(pntsd, *npntsd, numaces, acessize); + + copy_sec_desc(pntsd, *npntsd, numaces, acessize, bufsize); return 0; }