2

I am reading linux kernel 5.17.5 and now looking at container_of() macro.

/**
 * container_of - cast a member of a structure out to the containing structure
 * @ptr:    the pointer to the member.
 * @type:   the type of the container struct this is embedded in.
 * @member: the name of the member within the struct.
 *
 */
#define container_of(ptr, type, member) ({              \
    void *__mptr = (void *)(ptr);                   \
    static_assert(__same_type(*(ptr), ((type *)0)->member) ||   \
              __same_type(*(ptr), void),            \
              "pointer type mismatch in container_of()");   \
    ((type *)(__mptr - offsetof(type, member))); })

My question is really simple: What is the purpose of the __mptr, can i just replace __mptr by (void *)ptr like this?

#define container_of(ptr, type, member) ({          \
    static_assert(__same_type(*(ptr), ((type *)0)->member) ||   \
              __same_type(*(ptr), void),            \
              "pointer type mismatch in container_of()");   \
    ((type *)((void*)ptr - offsetof(type, member))); })
Chen Li
  • 4,824
  • 3
  • 28
  • 55
Puhan Zhou
  • 23
  • 3
  • Definitely not `(void *)ptr` but maybe `(void *)(ptr)`. But that will also cause problems as explained in Chen Li's answer. – Ian Abbott May 03 '22 at 11:14

1 Answers1

4

Since “kernel.h: handle pointers to arrays better in container_of()”, __mptr doesn't serve for type checking anymore. It is used to eliminate scripts/gcc-plugins/randomize_layout_plugin.c's informative message like inform(gimple_location(stmt), "found mismatched ssa struct pointer types: %qT and %qT\n", ptr_lhs_type, ptr_rhs_type);. One of this file's job is:

/*
 * iterate over all statements to find "bad" casts:
 * those where the address of the start of a structure is cast
 * to a pointer of a structure of a different type, or a
 * structure pointer type is cast to a different structure pointer type
 */

If __mptr is missing, the macro will include code as you said:

(type *)((void*)ptr - offsetof(type, member)))

(PS: char * is better here, because it is guaranteed by iso c standard to be 1 byte for offsetof, void * is guaranteed by gnu c only)

If offsetof gets zero, and ptr will contain the start address of struct type member, then this will be cast to be a totally different type: struct type *. This form malfunctions per scripts/gcc-plugins/randomize_layout_plugin.c and will be detected by it.

With the introduce of void *__mptr = (void *)(ptr);, the compiler doesn't know the type of __mptr anymore, so scripts/gcc-plugins/randomize_layout_plugin.c will not complain when casting the void * __mptr to (type *) You can see the real case and related fixed patch from https://lkml.org/lkml/2017/6/20/873


Below is original answer for kernel prior to 4.13 when __mptr was used for type checking:

Let's simplify the container_of and see this case:

#include <stdio.h>
#include <stddef.h>
#define container_of(ptr, type, member) ({          \
    const typeof( ((type *)0)->member ) *__mptr = (ptr);    \
    (type *)( (char *)__mptr - offsetof(type,member) );})
#define container_of_without_typechecking(ptr, type, member) ({         \
    (type *)( (char *)ptr - offsetof(type,member) );})
struct Foo
{
    int a;
};
int main(int argc, char *argv[]) {
    struct Foo foo;
    int *a;

    printf("foo addr: %p\n", &foo);
    a = &foo.a;
    printf("container of a: %p\n", container_of_without_typechecking((unsigned long long*)a, struct Foo, a));
    printf("typecheck: container of a: %p\n", container_of((unsigned long long*)a, struct Foo, a));
    return 0;
}

container_of_without_typechecking doesn't have __mptr but container_of does.

When compiling:

a.c: In function ‘main’:
a.c:13:55: warning: initialization of ‘const int *’ from incompatible pointer type ‘long long unsigned int *’ [-Wincompatible-pointer-types]
   13 |         const typeof( ((type *)0)->member ) *__mptr = (ptr);    \
      |                                                       ^
a.c:28:47: note: in expansion of macro ‘container_of’
   28 |     printf("typecheck: container of a: %p\n", container_of((unsigned long long*)a, struct Foo, a));
      |                                               ^~~~~~~~~~~~

As you can see, container_of throws an incompatible pointer type warning while container_of_without_typechecking does not, so it just for type checking.

Also, note that the Linux kernel regards this warning as an error:

KBUILD_CFLAGS   += $(call cc-option,-Werror=incompatible-pointer-types)

So, you will get an error instead of a warning if you pass through the wrong type to container_of.

Chen Li
  • 4,824
  • 3
  • 28
  • 55
  • Thanks for your answer:). Before linux-4.13, kernel use `__mptr` for type checking, but after 4.13, type checking has been moved into `BUILD_BUG_ON_MSG` macro and use `static_assert` for type checking after 5.16. So the `__mptr` is just a history? – Puhan Zhou May 01 '22 at 06:41
  • @PuhanZhou `__mptr` since 4.13 is used to eliminate informational message _note: found mismatched ssa struct pointer types: 'struct f2fs_inode_info' and 'struct inode'_ which _happen for all structures that have a zero offset between the member and the container structure, i.e. idential pointers_, you can see from here: https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1426986.html – Chen Li May 01 '22 at 07:18
  • Could you add some details about how `__mptr` is involved in the type checking at all? As it is `void*` it is rather not checking the type. The suggested replacement from the OP does the type checking but without storing the address in a temp variable. I can't really see how the main question is addressed here. – Gerhardh May 01 '22 at 11:07
  • 1
    I'm sorry for that. Pls see my new updates, which explain why `__mptr` is still needed now. – Chen Li May 01 '22 at 14:00
  • 1
    As a point of interest, the “kernel.h: handle pointers to arrays better in container_of()” patch was definitely submitted as a result of this https://stackoverflow.com/questions/39961293 – Ian Abbott May 03 '22 at 11:28
  • 1
    And my original version of the "kernel.h: handle pointers to arrays better in container_of()” patch submitted to LKML didn't have the intermediate `__mptr` variable, but that was merged in by Arnd Bergman before the patch made it to Linus's tree (see the msg1426986.html link you posted in the comments above). At least it still has my name on it. :-) (Maybe I submitted a "v2" patch - I can't remember!) – Ian Abbott May 03 '22 at 11:43
  • I guess the maintainer did this minor change and rebase your patch, which is kind of frequent. – Chen Li May 03 '22 at 12:37
  • 1
    @ChenLi Indeed, it made sense to combine the two patches into a single commit to make git bisecting easier. (Best to fix broken patches before they get passed upstream! I've done that myself too.) – Ian Abbott May 03 '22 at 17:08