[PATCH bpf 4/5] bpf: fix map permissions check
The map_lookup_and_delete_elem() function should check for both FMODE_CAN_WRITE
and FMODE_CAN_READ permissions because it returns a map element to user space.
Fixes: bd513cd08f10 ("bpf: add MAP_LOOKUP_AND_DELETE_ELEM syscall")
Signed-off-by: Anton Protopopov
---
kernel/bpf/syscall.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 4e6dee19a668..5e52765161f9 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1468,7 +1468,8 @@ static int map_lookup_and_delete_elem(union bpf_attr
*attr)
map = __bpf_map_get(f);
if (IS_ERR(map))
return PTR_ERR(map);
- if (!(map_get_sys_perms(map, f) & FMODE_CAN_WRITE)) {
+ if (!(map_get_sys_perms(map, f) & FMODE_CAN_READ) ||
+ !(map_get_sys_perms(map, f) & FMODE_CAN_WRITE)) {
err = -EPERM;
goto err_put;
}
--
2.20.1
[PATCH bpf 2/5] selftests/bpf: cleanup some file descriptors in test_maps
The test_map_rdonly and test_map_wronly tests should close file descriptors which they open. Signed-off-by: Anton Protopopov --- tools/testing/selftests/bpf/test_maps.c | 4 1 file changed, 4 insertions(+) diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c index f717acc0c68d..46cf2c232964 100644 --- a/tools/testing/selftests/bpf/test_maps.c +++ b/tools/testing/selftests/bpf/test_maps.c @@ -1401,6 +1401,8 @@ static void test_map_rdonly(void) /* Check that key=2 is not found. */ assert(bpf_map_lookup_elem(fd, , ) == -1 && errno == ENOENT); assert(bpf_map_get_next_key(fd, , ) == -1 && errno == ENOENT); + + close(fd); } static void test_map_wronly(void) @@ -1423,6 +1425,8 @@ static void test_map_wronly(void) /* Check that key=2 is not found. */ assert(bpf_map_lookup_elem(fd, , ) == -1 && errno == EPERM); assert(bpf_map_get_next_key(fd, , ) == -1 && errno == EPERM); + + close(fd); } static void prepare_reuseport_grp(int type, int map_fd, size_t map_elem_size, -- 2.20.1
[PATCH bpf 5/5] selftests/bpf: add tests for write-only stacks/queues
For write-only stacks and queues bpf_map_update_elem should be allowed, but
bpf_map_lookup_elem and bpf_map_lookup_and_delete_elem should fail with EPERM.
Signed-off-by: Anton Protopopov
---
tools/testing/selftests/bpf/test_maps.c | 40 -
1 file changed, 39 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/test_maps.c
b/tools/testing/selftests/bpf/test_maps.c
index 08d63948514a..6a12a0e01e07 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -1405,7 +1405,7 @@ static void test_map_rdonly(void)
close(fd);
}
-static void test_map_wronly(void)
+static void test_map_wronly_hash(void)
{
int fd, key = 0, value = 0;
@@ -1429,6 +1429,44 @@ static void test_map_wronly(void)
close(fd);
}
+static void test_map_wronly_stack_or_queue(enum bpf_map_type map_type)
+{
+ int fd, value = 0;
+
+ assert(map_type == BPF_MAP_TYPE_QUEUE ||
+ map_type == BPF_MAP_TYPE_STACK);
+ fd = bpf_create_map(map_type, 0, sizeof(value), MAP_SIZE,
+ map_flags | BPF_F_WRONLY);
+ /* Stack/Queue maps do not support BPF_F_NO_PREALLOC */
+ if (map_flags & BPF_F_NO_PREALLOC) {
+ assert(fd < 0 && errno == EINVAL);
+ return;
+ }
+ if (fd < 0) {
+ printf("Failed to create map '%s'!\n", strerror(errno));
+ exit(1);
+ }
+
+ value = 1234;
+ assert(bpf_map_update_elem(fd, NULL, , BPF_ANY) == 0);
+
+ /* Peek element should fail */
+ assert(bpf_map_lookup_elem(fd, NULL, ) == -1 && errno == EPERM);
+
+ /* Pop element should fail */
+ assert(bpf_map_lookup_and_delete_elem(fd, NULL, ) == -1 &&
+ errno == EPERM);
+
+ close(fd);
+}
+
+static void test_map_wronly(void)
+{
+ test_map_wronly_hash();
+ test_map_wronly_stack_or_queue(BPF_MAP_TYPE_STACK);
+ test_map_wronly_stack_or_queue(BPF_MAP_TYPE_QUEUE);
+}
+
static void prepare_reuseport_grp(int type, int map_fd, size_t map_elem_size,
__s64 *fds64, __u64 *sk_cookies,
unsigned int n)
--
2.20.1
[PATCH bpf 0/5] bpf: fix map permissions check and cleanup code around
This series fixes a bug in the map_lookup_and_delete_elem() function which should check for the FMODE_CAN_READ bit, because it returns data to user space. The rest of commits fix some typos and comment in selftests and extend the test_map_wronly test to cover the new check for the BPF_MAP_TYPE_STACK and BPF_MAP_TYPE_QUEUE map types. Anton Protopopov (5): selftests/bpf: fix a typo in test_maps selftests/bpf: cleanup some file descriptors in test_maps selftests/bpf: cleanup comments in test_maps bpf: fix map permissions check selftests/bpf: add tests for write-only stacks/queues kernel/bpf/syscall.c| 3 +- tools/testing/selftests/bpf/test_maps.c | 52 ++--- 2 files changed, 49 insertions(+), 6 deletions(-) -- 2.20.1
[PATCH bpf 3/5] selftests/bpf: cleanup comments in test_maps
Make comments inside the test_map_rdonly and test_map_wronly tests consistent with logic. Signed-off-by: Anton Protopopov --- tools/testing/selftests/bpf/test_maps.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c index 46cf2c232964..08d63948514a 100644 --- a/tools/testing/selftests/bpf/test_maps.c +++ b/tools/testing/selftests/bpf/test_maps.c @@ -1394,11 +1394,11 @@ static void test_map_rdonly(void) key = 1; value = 1234; - /* Insert key=1 element. */ + /* Try to insert key=1 element. */ assert(bpf_map_update_elem(fd, , , BPF_ANY) == -1 && errno == EPERM); - /* Check that key=2 is not found. */ + /* Check that key=1 is not found. */ assert(bpf_map_lookup_elem(fd, , ) == -1 && errno == ENOENT); assert(bpf_map_get_next_key(fd, , ) == -1 && errno == ENOENT); @@ -1422,7 +1422,7 @@ static void test_map_wronly(void) /* Insert key=1 element. */ assert(bpf_map_update_elem(fd, , , BPF_ANY) == 0); - /* Check that key=2 is not found. */ + /* Check that reading elements and keys from the map is not allowed. */ assert(bpf_map_lookup_elem(fd, , ) == -1 && errno == EPERM); assert(bpf_map_get_next_key(fd, , ) == -1 && errno == EPERM); -- 2.20.1
[PATCH bpf 1/5] selftests/bpf: fix a typo in test_maps
Trivial fix to a typo in the test_map_wronly test: "read" -> "write"
Signed-off-by: Anton Protopopov
---
tools/testing/selftests/bpf/test_maps.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/test_maps.c
b/tools/testing/selftests/bpf/test_maps.c
index c6766b2cff85..f717acc0c68d 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -1410,7 +1410,7 @@ static void test_map_wronly(void)
fd = bpf_create_map(BPF_MAP_TYPE_HASH, sizeof(key), sizeof(value),
MAP_SIZE, map_flags | BPF_F_WRONLY);
if (fd < 0) {
- printf("Failed to create map for read only test '%s'!\n",
+ printf("Failed to create map for write only test '%s'!\n",
strerror(errno));
exit(1);
}
--
2.20.1
Re: [PATCH bpf-next] tools: libbpf: update extended attributes version of bpf_object__open()
чт, 29 авг. 2019 г. в 16:02, Song Liu :
>
>
>
> > On Aug 14, 2019, at 5:03 PM, Anton Protopopov
> > wrote:
> >
>
> [...]
>
> >
> >
> > int bpf_object__unload(struct bpf_object *obj)
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index e8f70977d137..634f278578dd 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -63,8 +63,13 @@ LIBBPF_API libbpf_print_fn_t
> > libbpf_set_print(libbpf_print_fn_t fn);
> > struct bpf_object;
> >
> > struct bpf_object_open_attr {
> > - const char *file;
> > + union {
> > + const char *file;
> > + const char *obj_name;
> > + };
> > enum bpf_prog_type prog_type;
> > + void *obj_buf;
> > + size_t obj_buf_sz;
> > };
>
> I think this would break dynamically linked libbpf. No?
Ah, yes, sure. What is the right way to make changes which break ABI in libbpf?
BTW, does the commit ddc7c3042614 ("libbpf: implement BPF CO-RE offset
relocation algorithm") which adds a new field to the struct
bpf_object_load_attr also break ABI?
>
> Thanks,
> Song
>
[PATCH bpf-next] tools: libbpf: update extended attributes version of bpf_object__open()
Update the bpf_object_open_attr structure and corresponding code so that the
bpf_object__open_xattr function could be used to open objects from buffers as
well as from files. The reason for this change is that the existing
bpf_object__open_buffer function doesn't provide a way to specify neither the
needs_kver nor flags parameters to the internal call to __bpf_object__open
which makes it inconvenient for loading BPF objects which doesn't require a
kernel version.
Two new fields, obj_buf and obj_buf_sz, were added to the structure, and the
file field was union'ed with obj_name so that one can open an object like this:
struct bpf_object_open_attr attr = {
.obj_name = name,
.obj_buf= obj_buf,
.obj_buf_sz = obj_buf_sz,
.prog_type = BPF_PROG_TYPE_UNSPEC,
};
return bpf_object__open_xattr();
while still being able to use the file semantics:
struct bpf_object_open_attr attr = {
.file = path,
.prog_type = BPF_PROG_TYPE_UNSPEC,
};
return bpf_object__open_xattr();
Another thing to note is that since the commit c034a177d3c8 ("bpf: bpftool, add
flag to allow non-compat map definitions") which introduced a MAPS_RELAX_COMPAT
flag to load objects with non-compat map definitions, bpf_object__open_buffer
was called with this flag enabled (it was passed as the boolean true value in
flags argument to __bpf_object__open). The default behaviour for all open
functions is to clear this flag and this patch changes bpf_object__open_buffer
to clears this flag. It can be enabled, if needed, by opening an object from
buffer using __bpf_object__open_xattr.
Signed-off-by: Anton Protopopov
---
tools/lib/bpf/libbpf.c | 45 ++
tools/lib/bpf/libbpf.h | 7 ++-
2 files changed, 34 insertions(+), 18 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 2233f919dd88..7c8054afd901 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -3630,13 +3630,31 @@ __bpf_object__open(const char *path, void *obj_buf,
size_t obj_buf_sz,
struct bpf_object *__bpf_object__open_xattr(struct bpf_object_open_attr *attr,
int flags)
{
+ char tmp_name[64];
+
/* param validation */
- if (!attr->file)
+ if (!attr)
return NULL;
- pr_debug("loading %s\n", attr->file);
+ if (attr->obj_buf) {
+ if (attr->obj_buf_sz <= 0)
+ return NULL;
+ if (!attr->file) {
+ snprintf(tmp_name, sizeof(tmp_name), "%lx-%lx",
+(unsigned long)attr->obj_buf,
+(unsigned long)attr->obj_buf_sz);
+ attr->obj_name = tmp_name;
+ }
+ pr_debug("loading object '%s' from buffer\n", attr->obj_name);
+ } else if (!attr->file) {
+ return NULL;
+ } else {
+ attr->obj_buf_sz = 0;
- return __bpf_object__open(attr->file, NULL, 0,
+ pr_debug("loading object file '%s'\n", attr->file);
+ }
+
+ return __bpf_object__open(attr->file, attr->obj_buf, attr->obj_buf_sz,
bpf_prog_type__needs_kver(attr->prog_type),
flags);
}
@@ -3660,21 +3678,14 @@ struct bpf_object *bpf_object__open_buffer(void
*obj_buf,
size_t obj_buf_sz,
const char *name)
{
- char tmp_name[64];
-
- /* param validation */
- if (!obj_buf || obj_buf_sz <= 0)
- return NULL;
-
- if (!name) {
- snprintf(tmp_name, sizeof(tmp_name), "%lx-%lx",
-(unsigned long)obj_buf,
-(unsigned long)obj_buf_sz);
- name = tmp_name;
- }
- pr_debug("loading object '%s' from buffer\n", name);
+ struct bpf_object_open_attr attr = {
+ .obj_name = name,
+ .obj_buf= obj_buf,
+ .obj_buf_sz = obj_buf_sz,
+ .prog_type = BPF_PROG_TYPE_UNSPEC,
+ };
- return __bpf_object__open(name, obj_buf, obj_buf_sz, true, true);
+ return bpf_object__open_xattr();
}
int bpf_object__unload(struct bpf_object *obj)
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index e8f70977d137..634f278578dd 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -63,8 +63,13 @@ LIBBPF_API libbpf_print_fn_t
libbpf_set_print(libbpf_print_fn_t fn);
struct bpf_object;
struct bpf_object_open_attr {
- const char *file;
+ union {
+ const char *file;
+ const char *obj_name;
+ };
enum bpf_prog_typ
Re: [PATCH bpf-next 1/2] bpf, libbpf: add a new API bpf_object__reuse_maps()
вт, 9 июл. 2019 г. в 13:40, Andrii Nakryiko :
>
> On Mon, Jul 8, 2019 at 1:37 PM Anton Protopopov
> wrote:
> >
> > пн, 8 июл. 2019 г. в 13:54, Andrii Nakryiko :
> > >
> > > On Fri, Jul 5, 2019 at 2:53 PM Daniel Borkmann
> > > wrote:
> > > >
> > > > On 07/05/2019 10:44 PM, Anton Protopopov wrote:
> > > > > Add a new API bpf_object__reuse_maps() which can be used to replace
> > > > > all maps in
> > > > > an object by maps pinned to a directory provided in the path
> > > > > argument. Namely,
> > > > > each map M in the object will be replaced by a map pinned to
> > > > > path/M.name.
> > > > >
> > > > > Signed-off-by: Anton Protopopov
> > > > > ---
> > > > > tools/lib/bpf/libbpf.c | 34 ++
> > > > > tools/lib/bpf/libbpf.h | 2 ++
> > > > > tools/lib/bpf/libbpf.map | 1 +
> > > > > 3 files changed, 37 insertions(+)
> > > > >
> > > > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > > > index 4907997289e9..84c9e8f7bfd3 100644
> > > > > --- a/tools/lib/bpf/libbpf.c
> > > > > +++ b/tools/lib/bpf/libbpf.c
> > > > > @@ -3144,6 +3144,40 @@ int bpf_object__unpin_maps(struct bpf_object
> > > > > *obj, const char *path)
> > > > > return 0;
> > > > > }
> > > > >
> > > > > +int bpf_object__reuse_maps(struct bpf_object *obj, const char *path)
> > >
> > > As is, bpf_object__reuse_maps() can be easily implemented by user
> > > applications, as it's only using public libbpf APIs, so I'm not 100%
> > > sure we need to add method like that to libbpf.
> >
> > The bpf_object__reuse_maps() can definitely be implemented by user
> > applications, however, to use it a user also needs to re-implement the
> > bpf_prog_load_xattr funciton, so it seemed to me that adding this
> > functionality to the library is a better way.
>
> I'm still not convinced. Looking at bpf_prog_load_xattr, I think some
> of what it's doing should be part of bpf_object__object_xattr anyway
> (all the expected type setting for programs).
>
> Besides that, there isn't much more than just bpf_object__open and
> bpf_object__load, to be honest. By doing open and load explicitly,
> user gets an opportunity to do whatever adjustment they need: reuse
> maps, adjust map sizes, etc. So I think we should improve
> bpf_object__open to "guess" program attach types and add map
> definition flags to allow reuse declaratively.
>
>
> >
> > >
> > > > > +{
> > > > > + struct bpf_map *map;
> > > > > +
> > > > > + if (!obj)
> > > > > + return -ENOENT;
> > > > > +
> > > > > + if (!path)
> > > > > + return -EINVAL;
> > > > > +
> > > > > + bpf_object__for_each_map(map, obj) {
> > > > > + int len, err;
> > > > > + int pinned_map_fd;
> > > > > + char buf[PATH_MAX];
> > > >
> > > > We'd need to skip the case of bpf_map__is_internal(map) since they are
> > > > always
> > > > recreated for the given object.
> > > >
> > > > > + len = snprintf(buf, PATH_MAX, "%s/%s", path,
> > > > > bpf_map__name(map));
> > > > > + if (len < 0) {
> > > > > + return -EINVAL;
> > > > > + } else if (len >= PATH_MAX) {
> > > > > + return -ENAMETOOLONG;
> > > > > + }
> > > > > +
> > > > > + pinned_map_fd = bpf_obj_get(buf);
> > > > > + if (pinned_map_fd < 0)
> > > > > + return pinned_map_fd;
> > > >
> > > > Should we rather have a new map definition attribute that tells to reuse
> > > > the map if it's pinned in bpf fs, and if not, we create it and later on
> > > > pin it? This is what iproute2 is doing and which we're making use of
> > > > heavily.
> > >
> > > I'd like something like that as well. This would play nicely with
> > > recently added BTF-defined maps as well.
> > >
> > > I think it should be not just pin/don't pin flag, but rather
Re: [PATCH bpf-next 1/2] bpf, libbpf: add a new API bpf_object__reuse_maps()
пн, 8 июл. 2019 г. в 13:54, Andrii Nakryiko :
>
> On Fri, Jul 5, 2019 at 2:53 PM Daniel Borkmann wrote:
> >
> > On 07/05/2019 10:44 PM, Anton Protopopov wrote:
> > > Add a new API bpf_object__reuse_maps() which can be used to replace all
> > > maps in
> > > an object by maps pinned to a directory provided in the path argument.
> > > Namely,
> > > each map M in the object will be replaced by a map pinned to path/M.name.
> > >
> > > Signed-off-by: Anton Protopopov
> > > ---
> > > tools/lib/bpf/libbpf.c | 34 ++
> > > tools/lib/bpf/libbpf.h | 2 ++
> > > tools/lib/bpf/libbpf.map | 1 +
> > > 3 files changed, 37 insertions(+)
> > >
> > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > index 4907997289e9..84c9e8f7bfd3 100644
> > > --- a/tools/lib/bpf/libbpf.c
> > > +++ b/tools/lib/bpf/libbpf.c
> > > @@ -3144,6 +3144,40 @@ int bpf_object__unpin_maps(struct bpf_object *obj,
> > > const char *path)
> > > return 0;
> > > }
> > >
> > > +int bpf_object__reuse_maps(struct bpf_object *obj, const char *path)
>
> As is, bpf_object__reuse_maps() can be easily implemented by user
> applications, as it's only using public libbpf APIs, so I'm not 100%
> sure we need to add method like that to libbpf.
The bpf_object__reuse_maps() can definitely be implemented by user
applications, however, to use it a user also needs to re-implement the
bpf_prog_load_xattr funciton, so it seemed to me that adding this
functionality to the library is a better way.
>
> > > +{
> > > + struct bpf_map *map;
> > > +
> > > + if (!obj)
> > > + return -ENOENT;
> > > +
> > > + if (!path)
> > > + return -EINVAL;
> > > +
> > > + bpf_object__for_each_map(map, obj) {
> > > + int len, err;
> > > + int pinned_map_fd;
> > > + char buf[PATH_MAX];
> >
> > We'd need to skip the case of bpf_map__is_internal(map) since they are
> > always
> > recreated for the given object.
> >
> > > + len = snprintf(buf, PATH_MAX, "%s/%s", path,
> > > bpf_map__name(map));
> > > + if (len < 0) {
> > > + return -EINVAL;
> > > + } else if (len >= PATH_MAX) {
> > > + return -ENAMETOOLONG;
> > > + }
> > > +
> > > + pinned_map_fd = bpf_obj_get(buf);
> > > + if (pinned_map_fd < 0)
> > > + return pinned_map_fd;
> >
> > Should we rather have a new map definition attribute that tells to reuse
> > the map if it's pinned in bpf fs, and if not, we create it and later on
> > pin it? This is what iproute2 is doing and which we're making use of
> > heavily.
>
> I'd like something like that as well. This would play nicely with
> recently added BTF-defined maps as well.
>
> I think it should be not just pin/don't pin flag, but rather pinning
> strategy, to accommodate various typical strategies of handling maps
> that are already pinned. So something like this:
>
> 1. BPF_PIN_NOTHING - default, don't pin;
> 2. BPF_PIN_EXCLUSIVE - pin, but if map is already pinned - fail;
> 3. BPF_PIN_SET - pin; if existing map exists, reset its state to be
> exact state of object's map;
> 4. BPF_PIN_MERGE - pin, if map exists, fill in NULL entries only (this
> is how Cilium is pinning PROG_ARRAY maps, if I understand correctly);
> 5. BPF_PIN_MERGE_OVERWRITE - pin, if map exists, overwrite non-NULL values.
>
> This list is only for illustrative purposes, ideally people that have
> a lot of experience using pinning for real-world use cases would chime
> in on what strategies are useful and make sense.
My case was simply to reuse existing maps when reloading a program.
Does it make sense for you to add only the simplest cases of listed above?
Also, libbpf doesn't use standard naming conventions for pinning maps.
Does it make sense to provide a list of already open maps to the
bpf_prog_load_xattr function as an attribute? In this case a user
can execute his own policy on pinning, but still will have an option
to reuse, reset, and merge maps.
>
> > In bpf_object__reuse_maps() bailing out if bpf_obj_get() fails is perhaps
> > too limiting for a generic API as new version of an object file may contain
> > new maps which are not yet present in bpf fs at that point.
> >
> > > + err = bpf_map__reuse_fd(map, pi
Re: [PATCH bpf-next 1/2] bpf, libbpf: add a new API bpf_object__reuse_maps()
пт, 5 июл. 2019 г. в 17:44, Daniel Borkmann :
>
> On 07/05/2019 10:44 PM, Anton Protopopov wrote:
> > Add a new API bpf_object__reuse_maps() which can be used to replace all
> > maps in
> > an object by maps pinned to a directory provided in the path argument.
> > Namely,
> > each map M in the object will be replaced by a map pinned to path/M.name.
> >
> > Signed-off-by: Anton Protopopov
> > ---
> > tools/lib/bpf/libbpf.c | 34 ++
> > tools/lib/bpf/libbpf.h | 2 ++
> > tools/lib/bpf/libbpf.map | 1 +
> > 3 files changed, 37 insertions(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 4907997289e9..84c9e8f7bfd3 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -3144,6 +3144,40 @@ int bpf_object__unpin_maps(struct bpf_object *obj,
> > const char *path)
> > return 0;
> > }
> >
> > +int bpf_object__reuse_maps(struct bpf_object *obj, const char *path)
> > +{
> > + struct bpf_map *map;
> > +
> > + if (!obj)
> > + return -ENOENT;
> > +
> > + if (!path)
> > + return -EINVAL;
> > +
> > + bpf_object__for_each_map(map, obj) {
> > + int len, err;
> > + int pinned_map_fd;
> > + char buf[PATH_MAX];
>
> We'd need to skip the case of bpf_map__is_internal(map) since they are always
> recreated for the given object.
>
> > + len = snprintf(buf, PATH_MAX, "%s/%s", path,
> > bpf_map__name(map));
> > + if (len < 0) {
> > + return -EINVAL;
> > + } else if (len >= PATH_MAX) {
> > + return -ENAMETOOLONG;
> > + }
> > +
> > + pinned_map_fd = bpf_obj_get(buf);
> > + if (pinned_map_fd < 0)
> > + return pinned_map_fd;
>
> Should we rather have a new map definition attribute that tells to reuse
> the map if it's pinned in bpf fs, and if not, we create it and later on
> pin it? This is what iproute2 is doing and which we're making use of heavily.
What do you think about adding a new generic field, say load_flags,
to the bpf_map_def structure and a particular flag, say LOAD_F_STICKY
for this purpose? And it will be cleared for internal maps, so we will skip
them as well.
> In bpf_object__reuse_maps() bailing out if bpf_obj_get() fails is perhaps
> too limiting for a generic API as new version of an object file may contain
> new maps which are not yet present in bpf fs at that point.
How permissive should it be? Is it ok to just print a warning on any
bpf_obj_get()
failure? Or does it make sense to skip some specific error (ENOENT) and reject
on other errors?
>
> > + err = bpf_map__reuse_fd(map, pinned_map_fd);
> > + if (err)
> > + return err;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > int bpf_object__pin_programs(struct bpf_object *obj, const char *path)
> > {
> > struct bpf_program *prog;
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index d639f47e3110..7fe465a1be76 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -82,6 +82,8 @@ int bpf_object__variable_offset(const struct bpf_object
> > *obj, const char *name,
> > LIBBPF_API int bpf_object__pin_maps(struct bpf_object *obj, const char
> > *path);
> > LIBBPF_API int bpf_object__unpin_maps(struct bpf_object *obj,
> > const char *path);
> > +LIBBPF_API int bpf_object__reuse_maps(struct bpf_object *obj,
> > + const char *path);
> > LIBBPF_API int bpf_object__pin_programs(struct bpf_object *obj,
> > const char *path);
> > LIBBPF_API int bpf_object__unpin_programs(struct bpf_object *obj,
> > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> > index 2c6d835620d2..66a30be6696c 100644
> > --- a/tools/lib/bpf/libbpf.map
> > +++ b/tools/lib/bpf/libbpf.map
> > @@ -172,5 +172,6 @@ LIBBPF_0.0.4 {
> > btf_dump__new;
> > btf__parse_elf;
> > bpf_object__load_xattr;
> > + bpf_object__reuse_maps;
> > libbpf_num_possible_cpus;
> > } LIBBPF_0.0.3;
> >
>
[PATCH bpf-next 1/2] bpf, libbpf: add a new API bpf_object__reuse_maps()
Add a new API bpf_object__reuse_maps() which can be used to replace all maps in
an object by maps pinned to a directory provided in the path argument. Namely,
each map M in the object will be replaced by a map pinned to path/M.name.
Signed-off-by: Anton Protopopov
---
tools/lib/bpf/libbpf.c | 34 ++
tools/lib/bpf/libbpf.h | 2 ++
tools/lib/bpf/libbpf.map | 1 +
3 files changed, 37 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 4907997289e9..84c9e8f7bfd3 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -3144,6 +3144,40 @@ int bpf_object__unpin_maps(struct bpf_object *obj, const
char *path)
return 0;
}
+int bpf_object__reuse_maps(struct bpf_object *obj, const char *path)
+{
+ struct bpf_map *map;
+
+ if (!obj)
+ return -ENOENT;
+
+ if (!path)
+ return -EINVAL;
+
+ bpf_object__for_each_map(map, obj) {
+ int len, err;
+ int pinned_map_fd;
+ char buf[PATH_MAX];
+
+ len = snprintf(buf, PATH_MAX, "%s/%s", path,
bpf_map__name(map));
+ if (len < 0) {
+ return -EINVAL;
+ } else if (len >= PATH_MAX) {
+ return -ENAMETOOLONG;
+ }
+
+ pinned_map_fd = bpf_obj_get(buf);
+ if (pinned_map_fd < 0)
+ return pinned_map_fd;
+
+ err = bpf_map__reuse_fd(map, pinned_map_fd);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
int bpf_object__pin_programs(struct bpf_object *obj, const char *path)
{
struct bpf_program *prog;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index d639f47e3110..7fe465a1be76 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -82,6 +82,8 @@ int bpf_object__variable_offset(const struct bpf_object *obj,
const char *name,
LIBBPF_API int bpf_object__pin_maps(struct bpf_object *obj, const char *path);
LIBBPF_API int bpf_object__unpin_maps(struct bpf_object *obj,
const char *path);
+LIBBPF_API int bpf_object__reuse_maps(struct bpf_object *obj,
+ const char *path);
LIBBPF_API int bpf_object__pin_programs(struct bpf_object *obj,
const char *path);
LIBBPF_API int bpf_object__unpin_programs(struct bpf_object *obj,
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 2c6d835620d2..66a30be6696c 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -172,5 +172,6 @@ LIBBPF_0.0.4 {
btf_dump__new;
btf__parse_elf;
bpf_object__load_xattr;
+ bpf_object__reuse_maps;
libbpf_num_possible_cpus;
} LIBBPF_0.0.3;
--
2.19.1
[PATCH bpf-next 2/2] bpf, libbpf: add an option to reuse existing maps in bpf_prog_load_xattr
Add a new pinned_maps_path member to the bpf_prog_load_attr structure and
extend the bpf_prog_load_xattr() function to pass this pointer to the new
bpf_object__reuse_maps() helper. This change provides users with a simple
way to use existing pinned maps when (re)loading BPF programs.
Signed-off-by: Anton Protopopov
---
tools/lib/bpf/libbpf.c | 8
tools/lib/bpf/libbpf.h | 1 +
2 files changed, 9 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 84c9e8f7bfd3..9daa09c9fe1a 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -3953,6 +3953,14 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr
*attr,
first_prog = prog;
}
+ if (attr->pinned_maps_path) {
+ err = bpf_object__reuse_maps(obj, attr->pinned_maps_path);
+ if (err < 0) {
+ bpf_object__close(obj);
+ return err;
+ }
+ }
+
bpf_object__for_each_map(map, obj) {
if (!bpf_map__is_offload_neutral(map))
map->map_ifindex = attr->ifindex;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 7fe465a1be76..6bf405bb9c1f 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -329,6 +329,7 @@ struct bpf_prog_load_attr {
int ifindex;
int log_level;
int prog_flags;
+ const char *pinned_maps_path;
};
LIBBPF_API int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
--
2.19.1
[PATCH bpf-next 0/2] libbpf: add an option to reuse maps when loading a program
The following two patches add an option for users to reuse existing maps when loading a program using the bpf_prog_load_xattr function. A user can specify a directory containing pinned maps inside the bpf_prog_load_attr structure, and in this case the bpf_prog_load_xattr function will replace (bpf_map__reuse_fd) all maps defined in the object with file descriptors obtained from corresponding entries from the specified directory. Anton Protopopov (2): bpf, libbpf: add a new API bpf_object__reuse_maps() bpf, libbpf: add an option to reuse existing maps in bpf_prog_load_xattr tools/lib/bpf/libbpf.c | 42 tools/lib/bpf/libbpf.h | 3 +++ tools/lib/bpf/libbpf.map | 1 + 3 files changed, 46 insertions(+) -- 2.19.1
[PATCH bpf] bpf: fix the check that forwarding is enabled in bpf_ipv6_fib_lookup
The bpf_ipv6_fib_lookup function should return BPF_FIB_LKUP_RET_FWD_DISABLED
when forwarding is disabled for the input device. However instead of checking
if forwarding is enabled on the input device, it checked the global
net->ipv6.devconf_all->forwarding flag. Change it to behave as expected.
Signed-off-by: Anton Protopopov
---
net/core/filter.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/filter.c b/net/core/filter.c
index f615e42cf4ef..3fdf1b21be36 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4737,7 +4737,7 @@ static int bpf_ipv6_fib_lookup(struct net *net, struct
bpf_fib_lookup *params,
return -ENODEV;
idev = __in6_dev_get_safely(dev);
- if (unlikely(!idev || !net->ipv6.devconf_all->forwarding))
+ if (unlikely(!idev || !idev->cnf.forwarding))
return BPF_FIB_LKUP_RET_FWD_DISABLED;
if (flags & BPF_FIB_LOOKUP_OUTPUT) {
--
2.19.1
[PATCH] mISDN: prevent possible NULL pointer dereference
A return value of the bchannel_get_rxbuf() function is compared with the
positive ENOMEM value instead of the negative -ENOMEM value to detect a
memory allocation problem. Thus, after a possible memory allocation
failure the bc->bch.rx_skb will be NULL which will lead to a NULL
pointer dereference.
Signed-off-by: Anton Protopopov <a.s.protopo...@gmail.com>
---
drivers/isdn/hardware/mISDN/netjet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/isdn/hardware/mISDN/netjet.c
b/drivers/isdn/hardware/mISDN/netjet.c
index 8e29447..afde4ed 100644
--- a/drivers/isdn/hardware/mISDN/netjet.c
+++ b/drivers/isdn/hardware/mISDN/netjet.c
@@ -392,7 +392,7 @@ read_dma(struct tiger_ch *bc, u32 idx, int cnt)
}
stat = bchannel_get_rxbuf(>bch, cnt);
/* only transparent use the count here, HDLC overun is detected later */
- if (stat == ENOMEM) {
+ if (stat == -ENOMEM) {
pr_warning("%s.B%d: No memory for %d bytes\n",
card->name, bc->bch.nr, cnt);
return;
--
2.6.5
[PATCH] mISDN: prevent possible NULL pointer dereference
A return value of the bchannel_get_rxbuf() function is compared with the
positive ENOMEM value instead of the negative -ENOMEM value to detect a
memory allocation problem. Thus, after a possible memory allocation
failure the bc->bch.rx_skb will be NULL which will lead to a NULL
pointer dereference.
Signed-off-by: Anton Protopopov
---
drivers/isdn/hardware/mISDN/netjet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/isdn/hardware/mISDN/netjet.c
b/drivers/isdn/hardware/mISDN/netjet.c
index 8e29447..afde4ed 100644
--- a/drivers/isdn/hardware/mISDN/netjet.c
+++ b/drivers/isdn/hardware/mISDN/netjet.c
@@ -392,7 +392,7 @@ read_dma(struct tiger_ch *bc, u32 idx, int cnt)
}
stat = bchannel_get_rxbuf(>bch, cnt);
/* only transparent use the count here, HDLC overun is detected later */
- if (stat == ENOMEM) {
+ if (stat == -ENOMEM) {
pr_warning("%s.B%d: No memory for %d bytes\n",
card->name, bc->bch.nr, cnt);
return;
--
2.6.5
[PATCH] net: caif: fix erroneous return value
The cfrfml_receive() function might return positive value EPROTO Signed-off-by: Anton Protopopov <a.s.protopo...@gmail.com> --- net/caif/cfrfml.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/caif/cfrfml.c b/net/caif/cfrfml.c index 61d7617..b82440e 100644 --- a/net/caif/cfrfml.c +++ b/net/caif/cfrfml.c @@ -159,7 +159,7 @@ static int cfrfml_receive(struct cflayer *layr, struct cfpkt *pkt) tmppkt = NULL; /* Verify that length is correct */ - err = EPROTO; + err = -EPROTO; if (rfml->pdu_size != cfpkt_getlen(pkt) - RFM_HEAD_SIZE + 1) goto out; } -- 2.6.5
[PATCH] net: caif: fix erroneous return value
The cfrfml_receive() function might return positive value EPROTO Signed-off-by: Anton Protopopov --- net/caif/cfrfml.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/caif/cfrfml.c b/net/caif/cfrfml.c index 61d7617..b82440e 100644 --- a/net/caif/cfrfml.c +++ b/net/caif/cfrfml.c @@ -159,7 +159,7 @@ static int cfrfml_receive(struct cflayer *layr, struct cfpkt *pkt) tmppkt = NULL; /* Verify that length is correct */ - err = EPROTO; + err = -EPROTO; if (rfml->pdu_size != cfpkt_getlen(pkt) - RFM_HEAD_SIZE + 1) goto out; } -- 2.6.5
[PATCH] appletalk: fix erroneous return value
The atalk_sendmsg() function might return wrong value ENETUNREACH instead of -ENETUNREACH. Signed-off-by: Anton Protopopov <a.s.protopo...@gmail.com> --- net/appletalk/ddp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c index d5871ac..f066781 100644 --- a/net/appletalk/ddp.c +++ b/net/appletalk/ddp.c @@ -1625,7 +1625,7 @@ static int atalk_sendmsg(struct socket *sock, struct msghdr *msg, size_t len) rt = atrtr_find(_hint); } - err = ENETUNREACH; + err = -ENETUNREACH; if (!rt) goto out; -- 2.6.5
[PATCH] appletalk: fix erroneous return value
The atalk_sendmsg() function might return wrong value ENETUNREACH instead of -ENETUNREACH. Signed-off-by: Anton Protopopov --- net/appletalk/ddp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c index d5871ac..f066781 100644 --- a/net/appletalk/ddp.c +++ b/net/appletalk/ddp.c @@ -1625,7 +1625,7 @@ static int atalk_sendmsg(struct socket *sock, struct msghdr *msg, size_t len) rt = atrtr_find(_hint); } - err = ENETUNREACH; + err = -ENETUNREACH; if (!rt) goto out; -- 2.6.5
[PATCH] rtnl: RTM_GETNETCONF: fix wrong return value
An error response from a RTM_GETNETCONF request can return the positive error value EINVAL in the struct nlmsgerr that can mislead userspace. Signed-off-by: Anton Protopopov <a.s.protopo...@gmail.com> --- net/ipv4/devinet.c | 2 +- net/ipv6/addrconf.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c index cebd9d3..f6303b1 100644 --- a/net/ipv4/devinet.c +++ b/net/ipv4/devinet.c @@ -1847,7 +1847,7 @@ static int inet_netconf_get_devconf(struct sk_buff *in_skb, if (err < 0) goto errout; - err = EINVAL; + err = -EINVAL; if (!tb[NETCONFA_IFINDEX]) goto errout; diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 9efd9ff..bdd7eac 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -583,7 +583,7 @@ static int inet6_netconf_get_devconf(struct sk_buff *in_skb, if (err < 0) goto errout; - err = EINVAL; + err = -EINVAL; if (!tb[NETCONFA_IFINDEX]) goto errout; -- 2.6.5
[PATCH] rtnl: RTM_GETNETCONF: fix wrong return value
An error response from a RTM_GETNETCONF request can return the positive error value EINVAL in the struct nlmsgerr that can mislead userspace. Signed-off-by: Anton Protopopov --- net/ipv4/devinet.c | 2 +- net/ipv6/addrconf.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c index cebd9d3..f6303b1 100644 --- a/net/ipv4/devinet.c +++ b/net/ipv4/devinet.c @@ -1847,7 +1847,7 @@ static int inet_netconf_get_devconf(struct sk_buff *in_skb, if (err < 0) goto errout; - err = EINVAL; + err = -EINVAL; if (!tb[NETCONFA_IFINDEX]) goto errout; diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 9efd9ff..bdd7eac 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -583,7 +583,7 @@ static int inet6_netconf_get_devconf(struct sk_buff *in_skb, if (err < 0) goto errout; - err = EINVAL; + err = -EINVAL; if (!tb[NETCONFA_IFINDEX]) goto errout; -- 2.6.5
[PATCH] ext4: ioctl: fix erroneous return value
The ext4_ioctl_setflags() function which is used in the ioctls
EXT4_IOC_SETFLAGS and EXT4_IOC_FSSETXATTR may return the positive value
EPERM instead of -EPERM in case of error. This bug was introduced by a
recent commit 9b7365fc.
The following program can be used to illustrate the wrong behavior:
#include
#include
#include
#include
#include
#define FS_IOC_GETFLAGS _IOR('f', 1, long)
#define FS_IOC_SETFLAGS _IOW('f', 2, long)
#define FS_IMMUTABLE_FL 0x0010
int main(void)
{
int fd;
long flags;
fd = open("file", O_RDWR|O_CREAT, 0600);
if (fd < 0)
err(1, "open");
if (ioctl(fd, FS_IOC_GETFLAGS, ) < 0)
err(1, "ioctl: FS_IOC_GETFLAGS");
flags |= FS_IMMUTABLE_FL;
if (ioctl(fd, FS_IOC_SETFLAGS, ) < 0)
err(1, "ioctl: FS_IOC_SETFLAGS");
warnx("ioctl returned no error");
return 0;
}
Running it gives the following result:
$ strace -e ioctl ./test
ioctl(3, FS_IOC_GETFLAGS, 0x7ffdbd8bfd38) = 0
ioctl(3, FS_IOC_SETFLAGS, 0x7ffdbd8bfd38) = 1
test: ioctl returned no error
+++ exited with 0 +++
Running the program on a kernel with the bug fixed gives the proper result:
$ strace -e ioctl ./test
ioctl(3, FS_IOC_GETFLAGS, 0x7ffdd2768258) = 0
ioctl(3, FS_IOC_SETFLAGS, 0x7ffdd2768258) = -1 EPERM (Operation not
permitted)
test: ioctl: FS_IOC_SETFLAGS: Operation not permitted
+++ exited with 1 +++
Signed-off-by: Anton Protopopov
---
fs/ext4/ioctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index 0f6c369..a99b010 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -208,7 +208,7 @@ static int ext4_ioctl_setflags(struct inode *inode,
{
struct ext4_inode_info *ei = EXT4_I(inode);
handle_t *handle = NULL;
- int err = EPERM, migrate = 0;
+ int err = -EPERM, migrate = 0;
struct ext4_iloc iloc;
unsigned int oldflags, mask, i;
unsigned int jflag;
--
2.6.5
[PATCH] ext4: ioctl: fix erroneous return value
The ext4_ioctl_setflags() function which is used in the ioctls
EXT4_IOC_SETFLAGS and EXT4_IOC_FSSETXATTR may return the positive value
EPERM instead of -EPERM in case of error. This bug was introduced by a
recent commit 9b7365fc.
The following program can be used to illustrate the wrong behavior:
#include
#include
#include
#include
#include
#define FS_IOC_GETFLAGS _IOR('f', 1, long)
#define FS_IOC_SETFLAGS _IOW('f', 2, long)
#define FS_IMMUTABLE_FL 0x0010
int main(void)
{
int fd;
long flags;
fd = open("file", O_RDWR|O_CREAT, 0600);
if (fd < 0)
err(1, "open");
if (ioctl(fd, FS_IOC_GETFLAGS, ) < 0)
err(1, "ioctl: FS_IOC_GETFLAGS");
flags |= FS_IMMUTABLE_FL;
if (ioctl(fd, FS_IOC_SETFLAGS, ) < 0)
err(1, "ioctl: FS_IOC_SETFLAGS");
warnx("ioctl returned no error");
return 0;
}
Running it gives the following result:
$ strace -e ioctl ./test
ioctl(3, FS_IOC_GETFLAGS, 0x7ffdbd8bfd38) = 0
ioctl(3, FS_IOC_SETFLAGS, 0x7ffdbd8bfd38) = 1
test: ioctl returned no error
+++ exited with 0 +++
Running the program on a kernel with the bug fixed gives the proper result:
$ strace -e ioctl ./test
ioctl(3, FS_IOC_GETFLAGS, 0x7ffdd2768258) = 0
ioctl(3, FS_IOC_SETFLAGS, 0x7ffdd2768258) = -1 EPERM (Operation not
permitted)
test: ioctl: FS_IOC_SETFLAGS: Operation not permitted
+++ exited with 1 +++
Signed-off-by: Anton Protopopov <a.s.protopo...@gmail.com>
---
fs/ext4/ioctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index 0f6c369..a99b010 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -208,7 +208,7 @@ static int ext4_ioctl_setflags(struct inode *inode,
{
struct ext4_inode_info *ei = EXT4_I(inode);
handle_t *handle = NULL;
- int err = EPERM, migrate = 0;
+ int err = -EPERM, migrate = 0;
struct ext4_iloc iloc;
unsigned int oldflags, mask, i;
unsigned int jflag;
--
2.6.5
Re: [PATCH] cifs: fix erroneous return value
2016-02-10 12:59 GMT-05:00 Joe Perches :
> On Wed, 2016-02-10 at 12:50 -0500, Anton Protopopov wrote:
>> The setup_ntlmv2_rsp() function may return positive value ENOMEM instead
>> of -ENOMEM in case of kmalloc failure.
>
> How have you verified this change is correct?
Yes, this is the only case in which the setup_ntlmv2_rsp() function
returns positive error value. In all other error cases it returns the
negative error number.
>
> Have you checked that the callers of this function in
> fs/cifs/sess.c do the appropriate things with with a
> negative return value?
>
> The return value is now set into a struct member variable
> as a positive value,
>
> sess_data->result = rc;
As far as I can see, this member variable always contains a negative
error value which, in the end, is returned by the CIFS_SessSetup()
function (this function can also return -EINVAL, -ENOMEM, -EOPNOTSUPP,
etc.).
> Have you checked all the users of this member variable?
>
> If you have, you should say so in the commit message.
>
>> Signed-off-by: Anton Protopopov
>> ---
>> fs/cifs/cifsencrypt.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
>> index afa09fc..e682b36 100644
>> --- a/fs/cifs/cifsencrypt.c
>> +++ b/fs/cifs/cifsencrypt.c
>> @@ -714,7 +714,7 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct
>> nls_table *nls_cp)
>>
>> ses->auth_key.response = kmalloc(baselen + tilen, GFP_KERNEL);
>> if (!ses->auth_key.response) {
>> - rc = ENOMEM;
>> + rc = -ENOMEM;
>> ses->auth_key.len = 0;
>> goto setup_ntlmv2_rsp_ret;
>> }
>
[PATCH] cifs: fix erroneous return value
The setup_ntlmv2_rsp() function may return positive value ENOMEM instead
of -ENOMEM in case of kmalloc failure.
Signed-off-by: Anton Protopopov
---
fs/cifs/cifsencrypt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
index afa09fc..e682b36 100644
--- a/fs/cifs/cifsencrypt.c
+++ b/fs/cifs/cifsencrypt.c
@@ -714,7 +714,7 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct
nls_table *nls_cp)
ses->auth_key.response = kmalloc(baselen + tilen, GFP_KERNEL);
if (!ses->auth_key.response) {
- rc = ENOMEM;
+ rc = -ENOMEM;
ses->auth_key.len = 0;
goto setup_ntlmv2_rsp_ret;
}
--
2.6.5
[PATCH] ceph: fix a wrong comparison
A negative value rc compared to the positive value ENOENT in the
finish_read() function.
Signed-off-by: Anton Protopopov
---
fs/ceph/addr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index c222137..1b809c9 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -276,7 +276,7 @@ static void finish_read(struct ceph_osd_request *req,
struct ceph_msg *msg)
for (i = 0; i < num_pages; i++) {
struct page *page = osd_data->pages[i];
- if (rc < 0 && rc != ENOENT)
+ if (rc < 0 && rc != -ENOENT)
goto unlock;
if (bytes < (int)PAGE_CACHE_SIZE) {
/* zero (remainder of) page */
--
2.6.5
[PATCH] Bluetooth: hci_intel: Fix a wrong comparison
A return value of the intel_wait_booting() function compared with a constant ETIMEDOUT instead of -ETIMEDOUT. Signed-off-by: Anton Protopopov --- drivers/bluetooth/hci_intel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/bluetooth/hci_intel.c b/drivers/bluetooth/hci_intel.c index 3d63ea3..91d6051 100644 --- a/drivers/bluetooth/hci_intel.c +++ b/drivers/bluetooth/hci_intel.c @@ -488,7 +488,7 @@ static int intel_set_baudrate(struct hci_uart *hu, unsigned int speed) clear_bit(STATE_BOOTING, >flags); /* In case of timeout, try to continue anyway */ - if (err && err != ETIMEDOUT) + if (err && err != -ETIMEDOUT) return err; bt_dev_info(hdev, "Change controller speed to %d", speed); @@ -581,7 +581,7 @@ static int intel_setup(struct hci_uart *hu) clear_bit(STATE_BOOTING, >flags); /* In case of timeout, try to continue anyway */ - if (err && err != ETIMEDOUT) + if (err && err != -ETIMEDOUT) return err; set_bit(STATE_BOOTLOADER, >flags); -- 2.6.5
[PATCH] iwlwifi: fix erroneous return value
The iwl_trans_pcie_start_fw() function may return the positive value EIO
instead of -EIO in case of error.
Signed-off-by: Anton Protopopov
---
drivers/net/wireless/intel/iwlwifi/pcie/trans.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index d60a467..920ea9d 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -1034,7 +1034,7 @@ static int iwl_trans_pcie_start_fw(struct iwl_trans
*trans,
if (trans_pcie->is_down) {
IWL_WARN(trans,
"Can't start_fw since the HW hasn't been started\n");
- ret = EIO;
+ ret = -EIO;
goto out;
}
--
2.6.5
[PATCH] ath10k: fix erroneous return value
The ath10k_pci_hif_exchange_bmi_msg() function may return the positive
value EIO instead of -EIO in case of error.
Signed-off-by: Anton Protopopov
---
drivers/net/wireless/ath/ath10k/pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/pci.c
b/drivers/net/wireless/ath/ath10k/pci.c
index ee925c6..526acde 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1756,7 +1756,7 @@ static int ath10k_pci_hif_exchange_bmi_msg(struct ath10k
*ar,
DMA_FROM_DEVICE);
ret = dma_mapping_error(ar->dev, resp_paddr);
if (ret) {
- ret = EIO;
+ ret = -EIO;
goto err_req;
}
--
2.6.5
[PATCH] Bluetooth: hci_intel: Fix a wrong comparison
A return value of the intel_wait_booting() function compared with a constant ETIMEDOUT instead of -ETIMEDOUT. Signed-off-by: Anton Protopopov <a.s.protopo...@gmail.com> --- drivers/bluetooth/hci_intel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/bluetooth/hci_intel.c b/drivers/bluetooth/hci_intel.c index 3d63ea3..91d6051 100644 --- a/drivers/bluetooth/hci_intel.c +++ b/drivers/bluetooth/hci_intel.c @@ -488,7 +488,7 @@ static int intel_set_baudrate(struct hci_uart *hu, unsigned int speed) clear_bit(STATE_BOOTING, >flags); /* In case of timeout, try to continue anyway */ - if (err && err != ETIMEDOUT) + if (err && err != -ETIMEDOUT) return err; bt_dev_info(hdev, "Change controller speed to %d", speed); @@ -581,7 +581,7 @@ static int intel_setup(struct hci_uart *hu) clear_bit(STATE_BOOTING, >flags); /* In case of timeout, try to continue anyway */ - if (err && err != ETIMEDOUT) + if (err && err != -ETIMEDOUT) return err; set_bit(STATE_BOOTLOADER, >flags); -- 2.6.5
[PATCH] ceph: fix a wrong comparison
A negative value rc compared to the positive value ENOENT in the
finish_read() function.
Signed-off-by: Anton Protopopov <a.s.protopo...@gmail.com>
---
fs/ceph/addr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index c222137..1b809c9 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -276,7 +276,7 @@ static void finish_read(struct ceph_osd_request *req,
struct ceph_msg *msg)
for (i = 0; i < num_pages; i++) {
struct page *page = osd_data->pages[i];
- if (rc < 0 && rc != ENOENT)
+ if (rc < 0 && rc != -ENOENT)
goto unlock;
if (bytes < (int)PAGE_CACHE_SIZE) {
/* zero (remainder of) page */
--
2.6.5
[PATCH] iwlwifi: fix erroneous return value
The iwl_trans_pcie_start_fw() function may return the positive value EIO
instead of -EIO in case of error.
Signed-off-by: Anton Protopopov <a.s.protopo...@gmail.com>
---
drivers/net/wireless/intel/iwlwifi/pcie/trans.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index d60a467..920ea9d 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -1034,7 +1034,7 @@ static int iwl_trans_pcie_start_fw(struct iwl_trans
*trans,
if (trans_pcie->is_down) {
IWL_WARN(trans,
"Can't start_fw since the HW hasn't been started\n");
- ret = EIO;
+ ret = -EIO;
goto out;
}
--
2.6.5
[PATCH] ath10k: fix erroneous return value
The ath10k_pci_hif_exchange_bmi_msg() function may return the positive
value EIO instead of -EIO in case of error.
Signed-off-by: Anton Protopopov <a.s.protopo...@gmail.com>
---
drivers/net/wireless/ath/ath10k/pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/pci.c
b/drivers/net/wireless/ath/ath10k/pci.c
index ee925c6..526acde 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1756,7 +1756,7 @@ static int ath10k_pci_hif_exchange_bmi_msg(struct ath10k
*ar,
DMA_FROM_DEVICE);
ret = dma_mapping_error(ar->dev, resp_paddr);
if (ret) {
- ret = EIO;
+ ret = -EIO;
goto err_req;
}
--
2.6.5
Re: [PATCH] cifs: fix erroneous return value
2016-02-10 12:59 GMT-05:00 Joe Perches <j...@perches.com>:
> On Wed, 2016-02-10 at 12:50 -0500, Anton Protopopov wrote:
>> The setup_ntlmv2_rsp() function may return positive value ENOMEM instead
>> of -ENOMEM in case of kmalloc failure.
>
> How have you verified this change is correct?
Yes, this is the only case in which the setup_ntlmv2_rsp() function
returns positive error value. In all other error cases it returns the
negative error number.
>
> Have you checked that the callers of this function in
> fs/cifs/sess.c do the appropriate things with with a
> negative return value?
>
> The return value is now set into a struct member variable
> as a positive value,
>
> sess_data->result = rc;
As far as I can see, this member variable always contains a negative
error value which, in the end, is returned by the CIFS_SessSetup()
function (this function can also return -EINVAL, -ENOMEM, -EOPNOTSUPP,
etc.).
> Have you checked all the users of this member variable?
>
> If you have, you should say so in the commit message.
>
>> Signed-off-by: Anton Protopopov <a.s.protopo...@gmail.com>
>> ---
>> fs/cifs/cifsencrypt.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
>> index afa09fc..e682b36 100644
>> --- a/fs/cifs/cifsencrypt.c
>> +++ b/fs/cifs/cifsencrypt.c
>> @@ -714,7 +714,7 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct
>> nls_table *nls_cp)
>>
>> ses->auth_key.response = kmalloc(baselen + tilen, GFP_KERNEL);
>> if (!ses->auth_key.response) {
>> - rc = ENOMEM;
>> + rc = -ENOMEM;
>> ses->auth_key.len = 0;
>> goto setup_ntlmv2_rsp_ret;
>> }
>
[PATCH] cifs: fix erroneous return value
The setup_ntlmv2_rsp() function may return positive value ENOMEM instead
of -ENOMEM in case of kmalloc failure.
Signed-off-by: Anton Protopopov <a.s.protopo...@gmail.com>
---
fs/cifs/cifsencrypt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
index afa09fc..e682b36 100644
--- a/fs/cifs/cifsencrypt.c
+++ b/fs/cifs/cifsencrypt.c
@@ -714,7 +714,7 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct
nls_table *nls_cp)
ses->auth_key.response = kmalloc(baselen + tilen, GFP_KERNEL);
if (!ses->auth_key.response) {
- rc = ENOMEM;
+ rc = -ENOMEM;
ses->auth_key.len = 0;
goto setup_ntlmv2_rsp_ret;
}
--
2.6.5
[PATCH] netfilter: nft_counter: fix erroneous return values
The nft_counter_init() and nft_counter_clone() functions should return negative error value -ENOMEM instead of positive ENOMEM. Signed-off-by: Anton Protopopov --- net/netfilter/nft_counter.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/netfilter/nft_counter.c b/net/netfilter/nft_counter.c index c7808fc..c9743f7 100644 --- a/net/netfilter/nft_counter.c +++ b/net/netfilter/nft_counter.c @@ -100,7 +100,7 @@ static int nft_counter_init(const struct nft_ctx *ctx, cpu_stats = netdev_alloc_pcpu_stats(struct nft_counter_percpu); if (cpu_stats == NULL) - return ENOMEM; + return -ENOMEM; preempt_disable(); this_cpu = this_cpu_ptr(cpu_stats); @@ -138,7 +138,7 @@ static int nft_counter_clone(struct nft_expr *dst, const struct nft_expr *src) cpu_stats = __netdev_alloc_pcpu_stats(struct nft_counter_percpu, GFP_ATOMIC); if (cpu_stats == NULL) - return ENOMEM; + return -ENOMEM; preempt_disable(); this_cpu = this_cpu_ptr(cpu_stats); -- 2.1.4
[PATCH] media: i2c/adp1653: probe: fix erroneous return value
The adp1653_probe() function may return positive value EINVAL
which is obviously wrong.
Signed-off-by: Anton Protopopov
---
drivers/media/i2c/adp1653.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/i2c/adp1653.c b/drivers/media/i2c/adp1653.c
index 7e9cbf7..fb7ed73 100644
--- a/drivers/media/i2c/adp1653.c
+++ b/drivers/media/i2c/adp1653.c
@@ -497,7 +497,7 @@ static int adp1653_probe(struct i2c_client *client,
if (!client->dev.platform_data) {
dev_err(>dev,
"Neither DT not platform data provided\n");
- return EINVAL;
+ return -EINVAL;
}
flash->platform_data = client->dev.platform_data;
}
--
2.1.4
[PATCH] drm/qxl: fix erroneous return value
The qxl_gem_prime_mmap() function returns ENOSYS instead of -ENOSYS
Signed-off-by: Anton Protopopov
---
drivers/gpu/drm/qxl/qxl_prime.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/qxl/qxl_prime.c b/drivers/gpu/drm/qxl/qxl_prime.c
index 3d031b5..9f029dd 100644
--- a/drivers/gpu/drm/qxl/qxl_prime.c
+++ b/drivers/gpu/drm/qxl/qxl_prime.c
@@ -68,5 +68,5 @@ int qxl_gem_prime_mmap(struct drm_gem_object *obj,
struct vm_area_struct *area)
{
WARN_ONCE(1, "not implemented");
- return ENOSYS;
+ return -ENOSYS;
}
--
2.1.4
[PATCH] drm/qxl: fix erroneous return value
The qxl_gem_prime_mmap() function returns ENOSYS instead of -ENOSYS
Signed-off-by: Anton Protopopov <a.s.protopo...@gmail.com>
---
drivers/gpu/drm/qxl/qxl_prime.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/qxl/qxl_prime.c b/drivers/gpu/drm/qxl/qxl_prime.c
index 3d031b5..9f029dd 100644
--- a/drivers/gpu/drm/qxl/qxl_prime.c
+++ b/drivers/gpu/drm/qxl/qxl_prime.c
@@ -68,5 +68,5 @@ int qxl_gem_prime_mmap(struct drm_gem_object *obj,
struct vm_area_struct *area)
{
WARN_ONCE(1, "not implemented");
- return ENOSYS;
+ return -ENOSYS;
}
--
2.1.4
[PATCH] media: i2c/adp1653: probe: fix erroneous return value
The adp1653_probe() function may return positive value EINVAL
which is obviously wrong.
Signed-off-by: Anton Protopopov <a.s.protopo...@gmail.com>
---
drivers/media/i2c/adp1653.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/i2c/adp1653.c b/drivers/media/i2c/adp1653.c
index 7e9cbf7..fb7ed73 100644
--- a/drivers/media/i2c/adp1653.c
+++ b/drivers/media/i2c/adp1653.c
@@ -497,7 +497,7 @@ static int adp1653_probe(struct i2c_client *client,
if (!client->dev.platform_data) {
dev_err(>dev,
"Neither DT not platform data provided\n");
- return EINVAL;
+ return -EINVAL;
}
flash->platform_data = client->dev.platform_data;
}
--
2.1.4
[PATCH] netfilter: nft_counter: fix erroneous return values
The nft_counter_init() and nft_counter_clone() functions should return negative error value -ENOMEM instead of positive ENOMEM. Signed-off-by: Anton Protopopov <a.s.protopo...@gmail.com> --- net/netfilter/nft_counter.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/netfilter/nft_counter.c b/net/netfilter/nft_counter.c index c7808fc..c9743f7 100644 --- a/net/netfilter/nft_counter.c +++ b/net/netfilter/nft_counter.c @@ -100,7 +100,7 @@ static int nft_counter_init(const struct nft_ctx *ctx, cpu_stats = netdev_alloc_pcpu_stats(struct nft_counter_percpu); if (cpu_stats == NULL) - return ENOMEM; + return -ENOMEM; preempt_disable(); this_cpu = this_cpu_ptr(cpu_stats); @@ -138,7 +138,7 @@ static int nft_counter_clone(struct nft_expr *dst, const struct nft_expr *src) cpu_stats = __netdev_alloc_pcpu_stats(struct nft_counter_percpu, GFP_ATOMIC); if (cpu_stats == NULL) - return ENOMEM; + return -ENOMEM; preempt_disable(); this_cpu = this_cpu_ptr(cpu_stats); -- 2.1.4
[PATCH] Staging: comedi: check the return value of kobject_set_name
Added a check of the return value of the kobject_set_name function.
Signed-off-by: Anton Protopopov
---
drivers/staging/comedi/comedi_fops.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index 9d99fb3..dfeefa2 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2597,7 +2597,14 @@ static int __init comedi_init(void)
return -EIO;
cdev_init(_cdev, _fops);
comedi_cdev.owner = THIS_MODULE;
- kobject_set_name(_cdev.kobj, "comedi");
+
+ retval = kobject_set_name(_cdev.kobj, "comedi");
+ if (retval) {
+ unregister_chrdev_region(MKDEV(COMEDI_MAJOR, 0),
+COMEDI_NUM_MINORS);
+ return retval;
+ }
+
if (cdev_add(_cdev, MKDEV(COMEDI_MAJOR, 0), COMEDI_NUM_MINORS)) {
unregister_chrdev_region(MKDEV(COMEDI_MAJOR, 0),
COMEDI_NUM_MINORS);
--
1.8.3.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: comedi: check the return value of kobject_set_name
09.07.2014 08:28, gregkh пишет:
> On Wed, Jul 09, 2014 at 08:24:06AM +0400, Anton Protopopov wrote:
>> 2014-06-09 14:01 GMT+04:00 Ian Abbott :
>>> On 2014-06-07 14:56, Anton Protopopov wrote:
>>>>
>>>> Added a check of the return value of the kobject_set_name function.
>>>>
>>>> Signed-off-by: Anton Protopopov
>>>> ---
>>>> drivers/staging/comedi/comedi_fops.c | 9 -
>>>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>>
>>> Reviewed-by: Ian Abbott
>>
>> This patch was lost, I guess?
>
> I don't seem to have it anymore in my inbox, please resend.
>
> greg k-h
>
Added a check of the return value of the kobject_set_name function.
Signed-off-by: Anton Protopopov
---
drivers/staging/comedi/comedi_fops.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index 9d99fb3..dfeefa2 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2597,7 +2597,14 @@ static int __init comedi_init(void)
return -EIO;
cdev_init(_cdev, _fops);
comedi_cdev.owner = THIS_MODULE;
- kobject_set_name(_cdev.kobj, "comedi");
+
+ retval = kobject_set_name(_cdev.kobj, "comedi");
+ if (retval) {
+ unregister_chrdev_region(MKDEV(COMEDI_MAJOR, 0),
+COMEDI_NUM_MINORS);
+ return retval;
+ }
+
if (cdev_add(_cdev, MKDEV(COMEDI_MAJOR, 0), COMEDI_NUM_MINORS)) {
unregister_chrdev_region(MKDEV(COMEDI_MAJOR, 0),
COMEDI_NUM_MINORS);
--
1.8.3.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: comedi: check the return value of kobject_set_name
2014-06-09 14:01 GMT+04:00 Ian Abbott : > On 2014-06-07 14:56, Anton Protopopov wrote: >> >> Added a check of the return value of the kobject_set_name function. >> >> Signed-off-by: Anton Protopopov >> --- >> drivers/staging/comedi/comedi_fops.c | 9 - >> 1 file changed, 8 insertions(+), 1 deletion(-) > > > Reviewed-by: Ian Abbott This patch was lost, I guess? > > -- > -=( Ian Abbott @ MEV Ltd.E-mail: )=- > -=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: comedi: check the return value of kobject_set_name
2014-06-09 14:01 GMT+04:00 Ian Abbott abbo...@mev.co.uk: On 2014-06-07 14:56, Anton Protopopov wrote: Added a check of the return value of the kobject_set_name function. Signed-off-by: Anton Protopopov a.s.protopo...@gmail.com --- drivers/staging/comedi/comedi_fops.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) Reviewed-by: Ian Abbott abbo...@mev.co.uk This patch was lost, I guess? -- -=( Ian Abbott @ MEV Ltd.E-mail: abbo...@mev.co.uk)=- -=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: comedi: check the return value of kobject_set_name
09.07.2014 08:28, gregkh пишет:
On Wed, Jul 09, 2014 at 08:24:06AM +0400, Anton Protopopov wrote:
2014-06-09 14:01 GMT+04:00 Ian Abbott abbo...@mev.co.uk:
On 2014-06-07 14:56, Anton Protopopov wrote:
Added a check of the return value of the kobject_set_name function.
Signed-off-by: Anton Protopopov a.s.protopo...@gmail.com
---
drivers/staging/comedi/comedi_fops.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
Reviewed-by: Ian Abbott abbo...@mev.co.uk
This patch was lost, I guess?
I don't seem to have it anymore in my inbox, please resend.
greg k-h
Added a check of the return value of the kobject_set_name function.
Signed-off-by: Anton Protopopov a.s.protopo...@gmail.com
---
drivers/staging/comedi/comedi_fops.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index 9d99fb3..dfeefa2 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2597,7 +2597,14 @@ static int __init comedi_init(void)
return -EIO;
cdev_init(comedi_cdev, comedi_fops);
comedi_cdev.owner = THIS_MODULE;
- kobject_set_name(comedi_cdev.kobj, comedi);
+
+ retval = kobject_set_name(comedi_cdev.kobj, comedi);
+ if (retval) {
+ unregister_chrdev_region(MKDEV(COMEDI_MAJOR, 0),
+COMEDI_NUM_MINORS);
+ return retval;
+ }
+
if (cdev_add(comedi_cdev, MKDEV(COMEDI_MAJOR, 0), COMEDI_NUM_MINORS)) {
unregister_chrdev_region(MKDEV(COMEDI_MAJOR, 0),
COMEDI_NUM_MINORS);
--
1.8.3.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Staging: comedi: check the return value of kobject_set_name
Added a check of the return value of the kobject_set_name function.
Signed-off-by: Anton Protopopov a.s.protopo...@gmail.com
---
drivers/staging/comedi/comedi_fops.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index 9d99fb3..dfeefa2 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2597,7 +2597,14 @@ static int __init comedi_init(void)
return -EIO;
cdev_init(comedi_cdev, comedi_fops);
comedi_cdev.owner = THIS_MODULE;
- kobject_set_name(comedi_cdev.kobj, comedi);
+
+ retval = kobject_set_name(comedi_cdev.kobj, comedi);
+ if (retval) {
+ unregister_chrdev_region(MKDEV(COMEDI_MAJOR, 0),
+COMEDI_NUM_MINORS);
+ return retval;
+ }
+
if (cdev_add(comedi_cdev, MKDEV(COMEDI_MAJOR, 0), COMEDI_NUM_MINORS)) {
unregister_chrdev_region(MKDEV(COMEDI_MAJOR, 0),
COMEDI_NUM_MINORS);
--
1.8.3.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] staging: vt6655: fixed sparse warnings
This commit fixes a few sparse warnings for missing the 'static' keyword
in array definitions:
drivers/staging//vt6655/rxtx.c:81:22: warning: symbol 'wTimeStampOff' was
not declared. Should it be static?
drivers/staging//vt6655/rxtx.c:86:22: warning: symbol 'wFB_Opt0' was not
declared. Should it be static?
drivers/staging//vt6655/rxtx.c:90:22: warning: symbol 'wFB_Opt1' was not
declared. Should it be static?
Signed-off-by: Anton Protopopov
---
drivers/staging/vt6655/rxtx.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
index 2219d71..60fe6db 100644
--- a/drivers/staging/vt6655/rxtx.c
+++ b/drivers/staging/vt6655/rxtx.c
@@ -78,16 +78,16 @@ static int msglevel = MSG_LEVEL_INFO;
#define CRITICAL_PACKET_LEN 256// if packet size < 256 -> in-direct
send
//packet size >= 256 -> direct send
-const unsigned short wTimeStampOff[2][MAX_RATE] = {
+static const unsigned short wTimeStampOff[2][MAX_RATE] = {
{384, 288, 226, 209, 54, 43, 37, 31, 28, 25, 24, 23}, // Long Preamble
{384, 192, 130, 113, 54, 43, 37, 31, 28, 25, 24, 23}, // Short Preamble
};
-const unsigned short wFB_Opt0[2][5] = {
+static const unsigned short wFB_Opt0[2][5] = {
{RATE_12M, RATE_18M, RATE_24M, RATE_36M, RATE_48M}, // fallback_rate0
{RATE_12M, RATE_12M, RATE_18M, RATE_24M, RATE_36M}, // fallback_rate1
};
-const unsigned short wFB_Opt1[2][5] = {
+static const unsigned short wFB_Opt1[2][5] = {
{RATE_12M, RATE_18M, RATE_24M, RATE_24M, RATE_36M}, // fallback_rate0
{RATE_6M , RATE_6M, RATE_12M, RATE_12M, RATE_18M}, // fallback_rate1
};
--
1.8.3.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] staging: vt6655: fixed sparse warnings
This commit fixes a few sparse warnings for missing the 'static' keyword
in array definitions:
drivers/staging//vt6655/rxtx.c:81:22: warning: symbol 'wTimeStampOff' was
not declared. Should it be static?
drivers/staging//vt6655/rxtx.c:86:22: warning: symbol 'wFB_Opt0' was not
declared. Should it be static?
drivers/staging//vt6655/rxtx.c:90:22: warning: symbol 'wFB_Opt1' was not
declared. Should it be static?
Signed-off-by: Anton Protopopov a.s.protopo...@gmail.com
---
drivers/staging/vt6655/rxtx.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
index 2219d71..60fe6db 100644
--- a/drivers/staging/vt6655/rxtx.c
+++ b/drivers/staging/vt6655/rxtx.c
@@ -78,16 +78,16 @@ static int msglevel = MSG_LEVEL_INFO;
#define CRITICAL_PACKET_LEN 256// if packet size 256 - in-direct
send
//packet size = 256 - direct send
-const unsigned short wTimeStampOff[2][MAX_RATE] = {
+static const unsigned short wTimeStampOff[2][MAX_RATE] = {
{384, 288, 226, 209, 54, 43, 37, 31, 28, 25, 24, 23}, // Long Preamble
{384, 192, 130, 113, 54, 43, 37, 31, 28, 25, 24, 23}, // Short Preamble
};
-const unsigned short wFB_Opt0[2][5] = {
+static const unsigned short wFB_Opt0[2][5] = {
{RATE_12M, RATE_18M, RATE_24M, RATE_36M, RATE_48M}, // fallback_rate0
{RATE_12M, RATE_12M, RATE_18M, RATE_24M, RATE_36M}, // fallback_rate1
};
-const unsigned short wFB_Opt1[2][5] = {
+static const unsigned short wFB_Opt1[2][5] = {
{RATE_12M, RATE_18M, RATE_24M, RATE_24M, RATE_36M}, // fallback_rate0
{RATE_6M , RATE_6M, RATE_12M, RATE_12M, RATE_18M}, // fallback_rate1
};
--
1.8.3.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Staging: comedi: check the return value of kobject_set_name
Added a check of the return value of the kobject_set_name function.
Signed-off-by: Anton Protopopov
---
drivers/staging/comedi/comedi_fops.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index 9d99fb3..dfeefa2 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2597,7 +2597,14 @@ static int __init comedi_init(void)
return -EIO;
cdev_init(_cdev, _fops);
comedi_cdev.owner = THIS_MODULE;
- kobject_set_name(_cdev.kobj, "comedi");
+
+ retval = kobject_set_name(_cdev.kobj, "comedi");
+ if (retval) {
+ unregister_chrdev_region(MKDEV(COMEDI_MAJOR, 0),
+COMEDI_NUM_MINORS);
+ return retval;
+ }
+
if (cdev_add(_cdev, MKDEV(COMEDI_MAJOR, 0), COMEDI_NUM_MINORS)) {
unregister_chrdev_region(MKDEV(COMEDI_MAJOR, 0),
COMEDI_NUM_MINORS);
--
1.8.3.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Staging: comedi: check the return value of kobject_set_name
Added a check of the return value of the kobject_set_name function.
Signed-off-by: Anton Protopopov a.s.protopo...@gmail.com
---
drivers/staging/comedi/comedi_fops.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index 9d99fb3..dfeefa2 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2597,7 +2597,14 @@ static int __init comedi_init(void)
return -EIO;
cdev_init(comedi_cdev, comedi_fops);
comedi_cdev.owner = THIS_MODULE;
- kobject_set_name(comedi_cdev.kobj, comedi);
+
+ retval = kobject_set_name(comedi_cdev.kobj, comedi);
+ if (retval) {
+ unregister_chrdev_region(MKDEV(COMEDI_MAJOR, 0),
+COMEDI_NUM_MINORS);
+ return retval;
+ }
+
if (cdev_add(comedi_cdev, MKDEV(COMEDI_MAJOR, 0), COMEDI_NUM_MINORS)) {
unregister_chrdev_region(MKDEV(COMEDI_MAJOR, 0),
COMEDI_NUM_MINORS);
--
1.8.3.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
