nfsd: Prints a suitable length of filehandle
Subsystems (2)
PATCH 1/1 nfsd: Prints a suitable length of filehandle
With nfsd_debug on, fh_verify prints a fixed length of filehandle as,
nfsd: fh_verify(8: 00010001 00000000 00000000 00000000 00000000 00000000)
nfsd: fh_verify(28: 00070001 001c0002 00000000 041f9334 9b432a4c 835d4c95)
This patch lets nfsd prints a suitable length of filehandle as,
nfsd: fh_verify(8: 00010001 00000000)
nfsd: fh_verify(28: 00070001 001c0002 00000000 041f9334 9b432a4c 835d4c95 8cbea40f)
Also, use snprintf insteads sprintf.
--- fs/nfsd/nfsfh.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)
fs/nfsd/nfsfh.c +8 -10
@@ -650,16 +650,14 @@ fh_put(struct svc_fh *fhp)char *SVCFH_fmt (struct svc_fh *fhp) {struct knfsd_fh *fh = &fhp->fh_handle; - -static char buf[80 ]; -sprintf (buf,"%d: %08x %08x %08x %08x %08x %08x" , - fh->fh_size, - fh->fh_base.fh_pad[0 ], - fh->fh_base.fh_pad[1 ], - fh->fh_base.fh_pad[2 ], - fh->fh_base.fh_pad[3 ], - fh->fh_base.fh_pad[4 ], - fh->fh_base.fh_pad[5 ]); +/* 9 * NFS4_FHSIZE / 4 = 288 for fh_pad, 4 for fh_size, 1 for '\0' */ +static char buf[293 ]; +int pos =0 , i =0 ; + + pos =snprintf (buf,sizeof (buf),"%d:" , fh->fh_size); +for (i =0 ; i < fh->fh_size /sizeof (__u32); i++) + pos +=snprintf (buf + pos,sizeof (buf) - pos, +" %08x" , fh->fh_base.fh_pad[i]);return buf; }
Signature · 1 line
2.4.3
Quoted 25 lines · Kinglong Mee
With
nfsd_debugon,fh_verifyprints a fixed length of filehandle as,
nfsd:fh_verify(8: 00010001 00000000 00000000 00000000 00000000 00000000)
nfsd:fh_verify(28: 00070001 001c0002 00000000 041f9334 9b432a4c 835d4c95)This patch lets nfsd prints a suitable length of filehandle as,
nfsd:fh_verify(8: 00010001 00000000)
nfsd:fh_verify(28: 00070001 001c0002 00000000 041f9334 9b432a4c 835d4c95 8cbea40f)Also, use snprintf insteads sprintf.
Signed-off-by Kinglong Mee <[email protected]>--- fs/nfsd/nfsfh.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)fs/nfsd/nfsfh.c +0
-2@@ -650,16 +650,14 @@ fh_put(struct svc_fh *fhp)char *SVCFH_fmt (struct svc_fh *fhp) {struct knfsd_fh *fh = &fhp->fh_handle; - -static char buf[80 ];
fs/nfsd/nfsfh.c fh_put(struct svc_fh *fhp) · line 654
Quoted 10 lines
fs/nfsd/nfsfh.c +2
-8-sprintf (buf,"%d: %08x %08x %08x %08x %08x %08x" , - fh->fh_size, - fh->fh_base.fh_pad[0 ], - fh->fh_base.fh_pad[1 ], - fh->fh_base.fh_pad[2 ], - fh->fh_base.fh_pad[3 ], - fh->fh_base.fh_pad[4 ], - fh->fh_base.fh_pad[5 ]); +/* 9 * NFS4_FHSIZE / 4 = 288 for fh_pad, 4 for fh_size, 1 for '\0' */ +static char buf[293 ];
So that buffer can be shared by multiple users without any locking.
That's from the preexisting code, not your fault. Also, I don't know whether to care about an unlikely race in debugging code.
The only risk aside from corruption of the output might be if the race could leave the string temporarily non-null-terminated. I think that's possible? In which case that could be prevented by making sure buf is initialized to zeroes and is at least one byte longer than the following code will ever write.
--b.
Quoted 11 lines
fs/nfsd/nfsfh.c +6
-0+int pos =0 , i =0 ; + + pos =snprintf (buf,sizeof (buf),"%d:" , fh->fh_size); +for (i =0 ; i < fh->fh_size /sizeof (__u32); i++) + pos +=snprintf (buf + pos,sizeof (buf) - pos, +" %08x" , fh->fh_base.fh_pad[i]);return buf; }Signature · 1 line
2.4.3
Quoted 39 lines · J. Bruce Fields
Quoted 25 lines · Kinglong Mee
With
nfsd_debugon,fh_verifyprints a fixed length of filehandle as,
nfsd:fh_verify(8: 00010001 00000000 00000000 00000000 00000000 00000000)
nfsd:fh_verify(28: 00070001 001c0002 00000000 041f9334 9b432a4c 835d4c95)This patch lets nfsd prints a suitable length of filehandle as,
nfsd:fh_verify(8: 00010001 00000000)
nfsd:fh_verify(28: 00070001 001c0002 00000000 041f9334 9b432a4c 835d4c95 8cbea40f)Also, use snprintf insteads sprintf.
Signed-off-by Kinglong Mee <[email protected]>--- fs/nfsd/nfsfh.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)fs/nfsd/nfsfh.c +0
-2@@ -650,16 +650,14 @@ fh_put(struct svc_fh *fhp)char *SVCFH_fmt (struct svc_fh *fhp) {struct knfsd_fh *fh = &fhp->fh_handle; - -static char buf[80 ];fs/nfsd/nfsfh.c fh_put(struct svc_fh *fhp) · line 654
Quoted 10 lines
fs/nfsd/nfsfh.c +2
-8-sprintf (buf,"%d: %08x %08x %08x %08x %08x %08x" , - fh->fh_size, - fh->fh_base.fh_pad[0 ], - fh->fh_base.fh_pad[1 ], - fh->fh_base.fh_pad[2 ], - fh->fh_base.fh_pad[3 ], - fh->fh_base.fh_pad[4 ], - fh->fh_base.fh_pad[5 ]); +/* 9 * NFS4_FHSIZE / 4 = 288 for fh_pad, 4 for fh_size, 1 for '\0' */ +static char buf[293 ];So that buffer can be shared by multiple users without any locking.
Quoted 2 lines
That's from the preexisting code, not your fault. Also, I don't know whether to care about an unlikely race in debugging code.
I think we should not care it.
We cannot know the caller how to use it, printk directly or save to an pointer. Otherwise let the caller supporting the buffer? that sounds bad.
Quoted 6 lines
The only risk aside from corruption of the output might be if the race could leave the string temporarily non-null-terminated. I think that's possible? In which case that could be prevented by making sure buf is initialized to zeroes and is at least one byte longer than the following code will ever write.
Yes, It's possible.
Static variable will be initialized to all zero default. So, define a longer buffer is enough.
For that, I define a buffer with length is 293.
The max fh_size is NFS4_FHSIZE (128), as max length of decimal string with a ':' is 4.
The max fh_pad length should be
9 (%08x and a ' ') * NFS4_FHSIZE / 4 = 288.
At last, 1 for '\0'
thanks,
Kinglong Mee
Quoted 15 lines
--b.
Quoted 11 lines
fs/nfsd/nfsfh.c +6
-0+int pos =0 , i =0 ; + + pos =snprintf (buf,sizeof (buf),"%d:" , fh->fh_size); +for (i =0 ; i < fh->fh_size /sizeof (__u32); i++) + pos +=snprintf (buf + pos,sizeof (buf) - pos, +" %08x" , fh->fh_base.fh_pad[i]);return buf; }Signature · 1 line
2.4.3