diff mbox series

src/attr_replace_test: limit size of extended attribute value

Message ID 20190517120653.22180-1-lhenriques@suse.com (mailing list archive)
State New, archived
Headers show
Series src/attr_replace_test: limit size of extended attribute value | expand

Commit Message

Luis Henriques May 17, 2019, 12:06 p.m. UTC
The maximum size for extended attribute values is 65536 (XATTR_SIZE_MAX).
Since there are filesystems that can set blksize to really big values
(CephFS for example has a default of 4M), it's easy to have this test
failing with fsetxattr returning -E2BIG.

Cc: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Luis Henriques <lhenriques@suse.com>
---
 src/attr_replace_test.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Nikolay Borisov May 19, 2019, 5:27 p.m. UTC | #1
On 17.05.19 г. 15:06 ч., Luis Henriques wrote:
> The maximum size for extended attribute values is 65536 (XATTR_SIZE_MAX).
> Since there are filesystems that can set blksize to really big values
> (CephFS for example has a default of 4M), it's easy to have this test
> failing with fsetxattr returning -E2BIG.
> 
> Cc: Darrick J. Wong <darrick.wong@oracle.com>
> Signed-off-by: Luis Henriques <lhenriques@suse.com>
> ---
>  src/attr_replace_test.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/attr_replace_test.c b/src/attr_replace_test.c
> index 0720bfdc18ab..1ca9bf11ba58 100644
> --- a/src/attr_replace_test.c
> +++ b/src/attr_replace_test.c
> @@ -9,6 +9,7 @@
>  #include <sys/types.h>
>  #include <sys/xattr.h>
>  #include <sys/stat.h>
> +#include <linux/limits.h>
>  
>  #define die() do { perror(""); \
>  fprintf(stderr, "error at line %d\n", __LINE__); \
> @@ -44,6 +45,8 @@ int main(int argc, char *argv[])
>  	size = sbuf.st_blksize * 3 / 4;
>  	if (!size)
>  		fail("Invalid st_blksize(%ld)\n", sbuf.st_blksize);
> +	if (size > XATTR_SIZE_MAX)
> +		size = XATTR_SIZE_MAX;

nit: size = max(size, XATTRS_SIZE_MAX);

>  	value = malloc(size);
>  	if (!value)
>  		fail("Failed to allocate memory\n");
>
Luis Henriques May 20, 2019, 8:24 a.m. UTC | #2
Nikolay Borisov <nborisov@suse.com> writes:

> On 17.05.19 г. 15:06 ч., Luis Henriques wrote:
>> The maximum size for extended attribute values is 65536 (XATTR_SIZE_MAX).
>> Since there are filesystems that can set blksize to really big values
>> (CephFS for example has a default of 4M), it's easy to have this test
>> failing with fsetxattr returning -E2BIG.
>> 
>> Cc: Darrick J. Wong <darrick.wong@oracle.com>
>> Signed-off-by: Luis Henriques <lhenriques@suse.com>
>> ---
>>  src/attr_replace_test.c | 3 +++
>>  1 file changed, 3 insertions(+)
>> 
>> diff --git a/src/attr_replace_test.c b/src/attr_replace_test.c
>> index 0720bfdc18ab..1ca9bf11ba58 100644
>> --- a/src/attr_replace_test.c
>> +++ b/src/attr_replace_test.c
>> @@ -9,6 +9,7 @@
>>  #include <sys/types.h>
>>  #include <sys/xattr.h>
>>  #include <sys/stat.h>
>> +#include <linux/limits.h>
>>  
>>  #define die() do { perror(""); \
>>  fprintf(stderr, "error at line %d\n", __LINE__); \
>> @@ -44,6 +45,8 @@ int main(int argc, char *argv[])
>>  	size = sbuf.st_blksize * 3 / 4;
>>  	if (!size)
>>  		fail("Invalid st_blksize(%ld)\n", sbuf.st_blksize);
>> +	if (size > XATTR_SIZE_MAX)
>> +		size = XATTR_SIZE_MAX;
>
> nit: size = max(size, XATTRS_SIZE_MAX);

Thanks, Nikolay.  But I can't find the definition of such macro or
function.  I believe it's only available in kernel, right?  Also, you
probably meant min() ;-)

Cheers,
Nikolay Borisov May 20, 2019, 8:27 a.m. UTC | #3
On 20.05.19 г. 11:24 ч., Luis Henriques wrote:
> Nikolay Borisov <nborisov@suse.com> writes:
> 
>> On 17.05.19 г. 15:06 ч., Luis Henriques wrote:
>>> The maximum size for extended attribute values is 65536 (XATTR_SIZE_MAX).
>>> Since there are filesystems that can set blksize to really big values
>>> (CephFS for example has a default of 4M), it's easy to have this test
>>> failing with fsetxattr returning -E2BIG.
>>>
>>> Cc: Darrick J. Wong <darrick.wong@oracle.com>
>>> Signed-off-by: Luis Henriques <lhenriques@suse.com>
>>> ---
>>>  src/attr_replace_test.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/src/attr_replace_test.c b/src/attr_replace_test.c
>>> index 0720bfdc18ab..1ca9bf11ba58 100644
>>> --- a/src/attr_replace_test.c
>>> +++ b/src/attr_replace_test.c
>>> @@ -9,6 +9,7 @@
>>>  #include <sys/types.h>
>>>  #include <sys/xattr.h>
>>>  #include <sys/stat.h>
>>> +#include <linux/limits.h>
>>>  
>>>  #define die() do { perror(""); \
>>>  fprintf(stderr, "error at line %d\n", __LINE__); \
>>> @@ -44,6 +45,8 @@ int main(int argc, char *argv[])
>>>  	size = sbuf.st_blksize * 3 / 4;
>>>  	if (!size)
>>>  		fail("Invalid st_blksize(%ld)\n", sbuf.st_blksize);
>>> +	if (size > XATTR_SIZE_MAX)
>>> +		size = XATTR_SIZE_MAX;
>>
>> nit: size = max(size, XATTRS_SIZE_MAX);
> 
> Thanks, Nikolay.  But I can't find the definition of such macro or
> function.  I believe it's only available in kernel, right?  Also, you
> probably meant min() ;-)

Yeah, re min, but it seems there is a MIN/MAX macros provided in
<sys/param.h>. Anyway, as mentioned initially this is a minor point :) .

> 
> Cheers,
>
Darrick J. Wong May 20, 2019, 4:23 p.m. UTC | #4
On Fri, May 17, 2019 at 01:06:53PM +0100, Luis Henriques wrote:
> The maximum size for extended attribute values is 65536 (XATTR_SIZE_MAX).
> Since there are filesystems that can set blksize to really big values
> (CephFS for example has a default of 4M), it's easy to have this test
> failing with fsetxattr returning -E2BIG.
> 
> Cc: Darrick J. Wong <darrick.wong@oracle.com>
> Signed-off-by: Luis Henriques <lhenriques@suse.com>
> ---
>  src/attr_replace_test.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/attr_replace_test.c b/src/attr_replace_test.c
> index 0720bfdc18ab..1ca9bf11ba58 100644
> --- a/src/attr_replace_test.c
> +++ b/src/attr_replace_test.c
> @@ -9,6 +9,7 @@
>  #include <sys/types.h>
>  #include <sys/xattr.h>
>  #include <sys/stat.h>
> +#include <linux/limits.h>
>  
>  #define die() do { perror(""); \
>  fprintf(stderr, "error at line %d\n", __LINE__); \
> @@ -44,6 +45,8 @@ int main(int argc, char *argv[])
>  	size = sbuf.st_blksize * 3 / 4;
>  	if (!size)
>  		fail("Invalid st_blksize(%ld)\n", sbuf.st_blksize);
> +	if (size > XATTR_SIZE_MAX)
> +		size = XATTR_SIZE_MAX;

Looks good to me (this or the min()/MIN()/whatever version),

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D


>  	value = malloc(size);
>  	if (!value)
>  		fail("Failed to allocate memory\n");
diff mbox series

Patch

diff --git a/src/attr_replace_test.c b/src/attr_replace_test.c
index 0720bfdc18ab..1ca9bf11ba58 100644
--- a/src/attr_replace_test.c
+++ b/src/attr_replace_test.c
@@ -9,6 +9,7 @@ 
 #include <sys/types.h>
 #include <sys/xattr.h>
 #include <sys/stat.h>
+#include <linux/limits.h>
 
 #define die() do { perror(""); \
 fprintf(stderr, "error at line %d\n", __LINE__); \
@@ -44,6 +45,8 @@  int main(int argc, char *argv[])
 	size = sbuf.st_blksize * 3 / 4;
 	if (!size)
 		fail("Invalid st_blksize(%ld)\n", sbuf.st_blksize);
+	if (size > XATTR_SIZE_MAX)
+		size = XATTR_SIZE_MAX;
 	value = malloc(size);
 	if (!value)
 		fail("Failed to allocate memory\n");