Announce Push of V4.0.12
by Frank Filz
Branch next
Tag:V4.0.12
Merge Highlights
* More fixes of Coverity issues
* NFS: Add a config to limit the number of entries returned in readdir
* FSAL_PROXY_V3: Support POSIX attributes instead of ATTRS_NFS3
* SETATTR: FSALs were asking for FSAL_O_RDWR when truncating with a stateid
* Improve documentation of code submission process
* Other minor changes
Signed-off-by: Frank S. Filz <ffilzlnx(a)mindspring.com>
Contents:
b08457543 Frank S. Filz V4.0.12
6ac713c6c Frank S. Filz NFS: Add a config to limit the number of entries
returned in readdir
c529eaa4f Kaleb S. KEITHLEY Add pull_request_template.md with instructions
to submit patches to gerrithub instead
742252583 Paul Moyer fixed coverity issue 275287 (dead code) by ifdefing out
the unused code
c2d2aafb4 Paul Moyer fixed coverity complaints 132619, 132621, 132623,
313012 - rtc 867
ba2167bb3 Vicente Cheng doc: Add more description about Dir_Chunk
89f726750 Shahar Hochma libganesha_nfsd.ver: Export symbol
convert_ipv6_to_ipv4
9aec8e76d Assaf Yaari FSAL_PROXY_V3: Support POSIX attributes instead of
ATTRS_NFS3
e7efb5ee8 Frank S. Filz Update README.md to include contributing and
building references.
d571bd00f Frank S. Filz FSAL_MEM: fix comment typo
d5d822322 Frank S. Filz SETATTR: FSALs were asking for FSAL_O_RDWR when
truncating with a stateid
8bc76270d Frank S. Filz CID 356091 - fully set status in
nfs_readdir_dot_entry
1abd8d2c7 Frank S. Filz CID 361408 - ignore return from display_cat()
2 years, 1 month
Re: [S] Change in ...nfs-ganesha[next]: fixed coverity issue 275287 (dead code) by ifdefing out the unused code
by Frank Filz
I’m going to cc: the devel mailing list for more opinions
This code looks (mostly) safe to me. The concern I have is that it isn’t using gsh_malloc and gsh_free yet I assume that the buffer when freed later is freed with gsh_free.
Frank
From: Paul Moyer [mailto:Paul.Moyer@ibm.com]
Sent: Friday, November 4, 2022 7:55 AM
To: Frank Filz <ffilzlnx(a)mindspring.com>
Subject: RE: [S] Change in ...nfs-ganesha[next]: fixed coverity issue 275287 (dead code) by ifdefing out the unused code
After looking at this closer I don’t think it is a problem:
In function ‘xdr_utf8string_decode’,
inlined from ‘inline_xdr_utf8string’ at /home/moyer/275287/nfs-ganesha/src/include/nfsv41.h:4055:3,
inlined from ‘decode_acl’ at /home/moyer/275287/nfs-ganesha/src/Protocols/NFS/nfs_proto_tools.c:834:29:
/home/moyer/275287/nfs-ganesha/src/include/nfsv41.h:4041:8: warning: attempt to free a non-heap object ‘buffer’ [-Wfree-nonheap-object]
free(sp);
^
looking at the code (snipets)
3991 static inline bool
3992 xdr_utf8string_decode(XDR *xdrs, utf8string *objp, u_int maxsize)
3993 {
3994 /* sp is the actual string pointer */
3995 char *sp = objp->utf8string_val;
4026 if (!sp) {
4027 sp = (char *)malloc(size + 1);
4036 ret = xdr_opaque_decode(xdrs, sp, size);
4038 if (!ret) {
4039 if (!objp->utf8string_val) {
4040 /* Only free if we allocated */
4041 free(sp);
4042 }
4043 return ret;
4044 }
4045
4046 objp->utf8string_val = sp; /* only valid po inter */
so the flow seems to be:
sp is a pointer assigned to a string inside obj which is passed as a parameter
if sp is null, malloc is called and sp now points to “locally” allocated space
if an error occurs in xdr_opaque_decode(), enter the error path starting at 4038
if the string (in the passed object) that sp was originally assigned to is null, sp is free'd
therefore, sp is only malloc'ed if the original string is null, and if the original string is null, sp is free'd
which is consistent with the comment on 4040, though the logic took some study ...
the pointer in the passed object is subsequently replaced with sp:
if the pointer was not null to begin with, sp would already point to the same string
if the pointer was null, memory would be allocated and the passed object would be updated with a pointer to the “locally” allocated memory
i think this works because this method is declared as "static inline", so the allocated memory is ultimately local to the caller anyway ?
Let me know what you think.
From: Frank Filz <ffilzlnx(a)mindspring.com <mailto:ffilzlnx@mindspring.com> >
Sent: Thursday, November 3, 2022 3:07 PM
To: Paul Moyer <Paul.Moyer(a)ibm.com <mailto:Paul.Moyer@ibm.com> >
Subject: [EXTERNAL] RE: [S] Change in ...nfs-ganesha[next]: fixed coverity issue 275287 (dead code) by ifdefing out the unused code
Hmm, I’ve never seen that warning. And I didn’t realize we had code that did malloc/free… We should investigate that more closely. Frank From: Paul Moyer [mailto: Paul. Moyer@ ibm. com]
ZjQcmQRYFpfptBannerStart
This Message Is From an External Sender
This message came from outside your organization.
ZjQcmQRYFpfptBannerEnd
Hmm, I’ve never seen that warning.
And I didn’t realize we had code that did malloc/free…
We should investigate that more closely.
Frank
From: Paul Moyer [mailto:Paul.Moyer@ibm.com]
Sent: Thursday, November 3, 2022 11:19 AM
To: ffilzlnx(a)mindspring.com <mailto:ffilzlnx@mindspring.com>
Subject: RE: [S] Change in ...nfs-ganesha[next]: fixed coverity issue 275287 (dead code) by ifdefing out the unused code
Sorry for the email, but I have yet to figure out IRC.
While building the fix for 275287 I happened to notice a compiler warning:
[ 43%] Building C object Protocols/NFS/CMakeFiles/nfsproto.dir/nfs_proto_tools.c.o
In file included from /home/moyer/275287/nfs-ganesha/src/include/fsal_types.h:44:0,
from /home/moyer/275287/nfs-ganesha/src/include/fsal_api.h:45,
from /home/moyer/275287/nfs-ganesha/src/include/fsal.h:63,
from /home/moyer/275287/nfs-ganesha/src/Protocols/NFS/nfs_proto_tools.c:35:
In function ‘xdr_utf8string_decode’,
inlined from ‘inline_xdr_utf8string’ at /home/moyer/275287/nfs-ganesha/src/include/nfsv41.h:4055:3,
inlined from ‘decode_acl’ at /home/moyer/275287/nfs-ganesha/src/Protocols/NFS/nfs_proto_tools.c:834:29:
/home/moyer/275287/nfs-ganesha/src/include/nfsv41.h:4041:8: warning: attempt to free a non-heap object ‘buffer’ [-Wfree-nonheap-object]
free(sp);
^
There is even a comment in the code
4040 /* Only free if we allocated */
4041 free(sp);
Which makes me think someone else may have seen this before but decided not to fix it.
Anyway, should I fix this warning with 275287 ?
From: Frank Filz (GerritHub) <gerrit(a)gerrithub.io <mailto:gerrit@gerrithub.io> >
Sent: Thursday, November 3, 2022 1:29 PM
To: Paul Moyer <Paul.Moyer(a)ibm.com <mailto:Paul.Moyer@ibm.com> >
Cc: Daniel Gryniewicz <dang(a)redhat.com <mailto:dang@redhat.com> >; yogendra858(a)yahoo.com <mailto:yogendra858@yahoo.com> ; alokchandra.mallick(a)gmail.com <mailto:alokchandra.mallick@gmail.com> ; Gluster Community Jenkins <gerrithub(a)gluster.org <mailto:gerrithub@gluster.org> >
Subject: [EXTERNAL] [S] Change in ...nfs-ganesha[next]: fixed coverity issue 275287 (dead code) by ifdefing out the unused code
Attention is currently required from: Daniel Gryniewicz, yogendra858(a) yahoo. com, alokchandra. mallick(a) gmail. com, Paul Moyer. Patch set 1: Code-Review -1 View Change 1 comment: File src/SAL/recovery/recovery_fs_ng. c: Patch Set #1, Line 371:
ZjQcmQRYFpfptBannerStart
This Message Is From an External Sender
This message came from outside your organization.
ZjQcmQRYFpfptBannerEnd
Attention is currently required from: Daniel Gryniewicz, yogendra858(a)yahoo.com <mailto:yogendra858@yahoo.com> , alokchandra.mallick(a)gmail.com <mailto:alokchandra.mallick@gmail.com> , Paul Moyer.
Patch set 1:Code-Review -1
View Change <https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/545670>
1 comment:
* File src/SAL/recovery/recovery_fs_ng.c:
* Patch Set #1, Line 371: <https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/545670/comment/4e74c97b...> {
oops, need to ifdef out rc...
To view, visit change 545670 <https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/545670> . To unsubscribe, or for help writing mail filters, visit settings <https://review.gerrithub.io/settings> .
Gerrit-Project: ffilz/nfs-ganesha
Gerrit-Branch: next
Gerrit-Change-Id: I1fa6854136986774cc8f9cb87371f5709cbef935
Gerrit-Change-Number: 545670
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Moyer <paul.moyer(a)ibm.com <mailto:paul.moyer@ibm.com> >
Gerrit-Reviewer: Daniel Gryniewicz <dang(a)redhat.com <mailto:dang@redhat.com> >
Gerrit-Reviewer: Frank Filz <ffilzlnx(a)mindspring.com <mailto:ffilzlnx@mindspring.com> >
Gerrit-Reviewer: Gluster Community Jenkins <gerrithub(a)gluster.org <mailto:gerrithub@gluster.org> >
Gerrit-Reviewer: alokchandra.mallick(a)gmail.com <mailto:alokchandra.mallick@gmail.com>
Gerrit-Reviewer: yogendra858(a)yahoo.com <mailto:yogendra858@yahoo.com>
Gerrit-Attention: Daniel Gryniewicz <dang(a)redhat.com <mailto:dang@redhat.com> >
Gerrit-Attention: yogendra858(a)yahoo.com <mailto:yogendra858@yahoo.com>
Gerrit-Attention: alokchandra.mallick(a)gmail.com <mailto:alokchandra.mallick@gmail.com>
Gerrit-Attention: Paul Moyer <paul.moyer(a)ibm.com <mailto:paul.moyer@ibm.com> >
Gerrit-Comment-Date: Thu, 03 Nov 2022 17:29:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
2 years, 1 month
[S] Change in ...nfs-ganesha[next]: SETATTR: FSALs were asking for FSAL_O_RDWR when truncating with a sta...
by Frank Filz (GerritHub)
Frank Filz has uploaded this change for review. ( https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/545617 )
Change subject: SETATTR: FSALs were asking for FSAL_O_RDWR when truncating with a stateid
......................................................................
SETATTR: FSALs were asking for FSAL_O_RDWR when truncating with a stateid
We only need FSAL_O_WRITE when we have a stateid.
Failing this, we opened the global fd and that then prevented local fs
execution of a binary that had been copied.
Change-Id: Ic476ddf529ba0c45c4d83a4150b4181166a3330a
Signed-off-by: Frank S. Filz <ffilzlnx(a)mindspring.com>
---
M src/FSAL/FSAL_CEPH/handle.c
M src/FSAL/FSAL_GLUSTER/handle.c
M src/FSAL/FSAL_LIZARDFS/handle.c
M src/FSAL/FSAL_RGW/handle.c
M src/FSAL/FSAL_VFS/file.c
5 files changed, 20 insertions(+), 5 deletions(-)
git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha refs/changes/17/545617/1
--
To view, visit https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/545617
To unsubscribe, or for help writing mail filters, visit https://review.gerrithub.io/settings
Gerrit-Project: ffilz/nfs-ganesha
Gerrit-Branch: next
Gerrit-Change-Id: Ic476ddf529ba0c45c4d83a4150b4181166a3330a
Gerrit-Change-Number: 545617
Gerrit-PatchSet: 1
Gerrit-Owner: Frank Filz <ffilzlnx(a)mindspring.com>
Gerrit-MessageType: newchange
2 years, 1 month
[S] Change in ...nfs-ganesha[next]: fixed coverity complaints 132619, 132621, 132623, 313012 - rtc 867
by Paul Moyer (GerritHub)
Paul Moyer has uploaded this change for review. ( https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/545549 )
Change subject: fixed coverity complaints 132619, 132621, 132623, 313012 - rtc 867
......................................................................
fixed coverity complaints 132619, 132621, 132623, 313012 - rtc 867
Change-Id: I23e6013c2d867143945c7e0e9b652948b0517ac7
Signed-off-by: Paul Moyer <paul.moyer(a)ibm.com>
---
M src/Protocols/NFS/nfs_proto_tools.c
M src/Protocols/XDR/xdr_nfsacl.c
2 files changed, 20 insertions(+), 6 deletions(-)
git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha refs/changes/49/545549/1
--
To view, visit https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/545549
To unsubscribe, or for help writing mail filters, visit https://review.gerrithub.io/settings
Gerrit-Project: ffilz/nfs-ganesha
Gerrit-Branch: next
Gerrit-Change-Id: I23e6013c2d867143945c7e0e9b652948b0517ac7
Gerrit-Change-Number: 545549
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Moyer <paul.moyer(a)ibm.com>
Gerrit-MessageType: newchange
2 years, 1 month