| Message ID | 20240125145128.12945-1-mora@netapp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | NFSv4.2: fix listxattr maximum XDR buffer size | expand |
On 25 Jan 2024, at 9:51, Jorge Mora wrote: > Switch order of operations to avoid creating a short XDR buffer: > e.g., buflen = 12, old xdrlen = 12, new xdrlen = 20. > > Having a short XDR buffer leads to lxa_maxcount be a few bytes > less than what is needed to retrieve the whole list when using > a buflen as returned by a call with size = 0: > buflen = listxattr(path, NULL, 0); > buf = malloc(buflen); > buflen = listxattr(path, buf, buflen); > > For a file with one attribute (name = '123456'), the first call > with size = 0 will return buflen = 12 ('user.123456\x00'). > The second call with size = 12, sends LISTXATTRS with > lxa_maxcount = 12 + 8 (cookie) + 4 (array count) = 24. The > XDR buffer needs 8 (cookie) + 4 (array count) + 4 (name count) > + 6 (name len) + 2 (padding) + 4 (eof) = 28 which is 4 bytes > shorter than the lxa_maxcount provided in the call. > > Fixes: 04a5da690e8f ("NFSv4.2: define limits and sizes for user xattr handling") > Signed-off-by: Jorge Mora <mora@netapp.com> > --- > fs/nfs/nfs42.h | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/fs/nfs/nfs42.h b/fs/nfs/nfs42.h > index b59876b01a1e..32ee9ad6a560 100644 > --- a/fs/nfs/nfs42.h > +++ b/fs/nfs/nfs42.h > @@ -55,11 +55,14 @@ int nfs42_proc_removexattr(struct inode *inode, const char *name); > * They would be 7 bytes long in the eventual buffer ("user.x\0"), and > * 8 bytes long XDR-encoded. > * > - * Include the trailing eof word as well. > + * Include the trailing eof word as well and make the result a multiple > + * of 4 bytes. > */ > static inline u32 nfs42_listxattr_xdrsize(u32 buflen) > { > - return ((buflen / (XATTR_USER_PREFIX_LEN + 2)) * 8) + 4; > + u32 size = 8 * buflen / (XATTR_USER_PREFIX_LEN + 2) + 4; > + > + return (size + 3) & ~3; Maybe XDR_QUADLEN(size) here? Otherwise, looks correct. Reviewed-by: Benjamin Coddington <bcodding@redhat.com> Ben
On 26 Jan 2024, at 15:37, Jorge Mora wrote: > Thanks Ben, > > Do you want me to resubmit the patch using XDR_QUALEN? Something like this? > > static inline u32 nfs42_listxattr_xdrsize(u32 buflen) > { > - return ((buflen / (XATTR_USER_PREFIX_LEN + 2)) * 8) + 4; > + u32 size = 8 * buflen / (XATTR_USER_PREFIX_LEN + 2) + 4; > + > + return XDR_QUADLEN(size) << 2; I just thought I'd mention it since it's used all over. I think your version is just fine. Ben
diff --git a/fs/nfs/nfs42.h b/fs/nfs/nfs42.h index b59876b01a1e..32ee9ad6a560 100644 --- a/fs/nfs/nfs42.h +++ b/fs/nfs/nfs42.h @@ -55,11 +55,14 @@ int nfs42_proc_removexattr(struct inode *inode, const char *name); * They would be 7 bytes long in the eventual buffer ("user.x\0"), and * 8 bytes long XDR-encoded. * - * Include the trailing eof word as well. + * Include the trailing eof word as well and make the result a multiple + * of 4 bytes. */ static inline u32 nfs42_listxattr_xdrsize(u32 buflen) { - return ((buflen / (XATTR_USER_PREFIX_LEN + 2)) * 8) + 4; + u32 size = 8 * buflen / (XATTR_USER_PREFIX_LEN + 2) + 4; + + return (size + 3) & ~3; } #endif /* CONFIG_NFS_V4_2 */ #endif /* __LINUX_FS_NFS_NFS4_2_H */
Switch order of operations to avoid creating a short XDR buffer: e.g., buflen = 12, old xdrlen = 12, new xdrlen = 20. Having a short XDR buffer leads to lxa_maxcount be a few bytes less than what is needed to retrieve the whole list when using a buflen as returned by a call with size = 0: buflen = listxattr(path, NULL, 0); buf = malloc(buflen); buflen = listxattr(path, buf, buflen); For a file with one attribute (name = '123456'), the first call with size = 0 will return buflen = 12 ('user.123456\x00'). The second call with size = 12, sends LISTXATTRS with lxa_maxcount = 12 + 8 (cookie) + 4 (array count) = 24. The XDR buffer needs 8 (cookie) + 4 (array count) + 4 (name count) + 6 (name len) + 2 (padding) + 4 (eof) = 28 which is 4 bytes shorter than the lxa_maxcount provided in the call. Fixes: 04a5da690e8f ("NFSv4.2: define limits and sizes for user xattr handling") Signed-off-by: Jorge Mora <mora@netapp.com> --- fs/nfs/nfs42.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)