Re: fuse: Avoid fuse_file_args null pointer dereference

[Bernd Schubert <bernd.schubert@xxxxxxxxxxx>]

On 4/23/24 12:46, Miklos Szeredi wrote:
> On Mon, 22 Apr 2024 at 19:40, Bernd Schubert <bernd.schubert@xxxxxxxxxxx> wrote:
>>
>> The test for NULL was done for the member of union fuse_file_args,
>> but not for fuse_file_args itself.
>>
>> Fixes: e26ee4efbc796 ("fuse: allocate ff->release_args only if release is needed")
>> Signed-off-by: Bernd Schubert <bschubert@xxxxxxx>
>>
>> ---
>> I'm currently going through all the recent patches again and noticed
>> in code review. I guess this falls through testing, because we don't
>> run xfstests that have !fc->no_opendir || !fc->no_open.
>>
>> Note: Untested except that it compiles.
>>
>> Note2: Our IT just broke sendmail, I'm quickly sending through thunderbird,
>> I hope doesn't change the patch format.
>>
>>   fs/fuse/file.c |    7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
>> index b57ce4157640..0ff865457ea6 100644
>> --- a/fs/fuse/file.c
>> +++ b/fs/fuse/file.c
>> @@ -102,7 +102,8 @@ static void fuse_release_end(struct fuse_mount *fm, struct fuse_args *args,
>>   static void fuse_file_put(struct fuse_file *ff, bool sync)
>>   {
>>         if (refcount_dec_and_test(&ff->count)) {
>> -               struct fuse_release_args *ra = &ff->args->release_args;
>> +               struct fuse_release_args *ra =
>> +                       ff->args ? &ff->args->release_args : NULL;
> 
> While this looks like a NULL pointer dereference, it isn't, because
> &foo->bar is just pointer arithmetic, and in this case the pointers
> will be identical.  So it will work, but the whole ff->args thing is a
> bit confusing.   Not sure how to properly clean this up, your patch
> seems to be just adding more obfuscation.

Hmm, right, I had actually thought about that and written a small test,
before creating the patch. But then had it slightly different - caused
the null-ptr deref.
Updated code works, but UBSAN still complains.


bschubert2@imesrv6 test>./test-union
test-union.c:23:10: runtime error: member access within null pointer of
type 'union bar'
No ptr


cat test-union.c

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <assert.h>

union bar
{
    int foo;
    int foo2;
};

struct test
{
    union bar *bar;
};

int main(void)
{
    struct test *test = calloc(1, sizeof(test));
    assert(test);

    int *foo_ptr = &test->bar->foo;

    if (foo_ptr)
        printf("Have ptr\n");
    else
        printf("No ptr\n");

    free(test);

    return 0;
}



Thanks,
Bernd