Re: [ovs-dev] [PATCH v2 2/2] net: openvswitch: Annotate struct mask_array with __counted_by
On Sat, 14 Oct 2023, Kees Cook wrote:
> On Sat, Oct 14, 2023 at 08:34:53AM +0200, Christophe JAILLET wrote:
> > Prepare for the coming implementation by GCC and Clang of the __counted_by
> > attribute. Flexible array members annotated with __counted_by can have
> > their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
> > (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
> > functions).
> >
> > Signed-off-by: Christophe JAILLET
> > ---
> > v2: Fix the subject [Ilya Maximets]
> > fix the field name used with __counted_by [Ilya Maximets]
> >
> > v1:
> > https://lore.kernel.org/all/f66ddcf1ef9328f10292ea75a17b584359b6cde3.1696156198.git.christophe.jail...@wanadoo.fr/
> >
> >
> > This patch is part of a work done in parallel of what is currently worked
> > on by Kees Cook.
> >
> > My patches are only related to corner cases that do NOT match the
> > semantic of his Coccinelle script[1].
What was the problem with the semantic patch in this case?
thanks,
julia
> >
> > In this case, in tbl_mask_array_alloc(), several things are allocated with
> > a single allocation. Then, some pointer arithmetic computes the address of
> > the memory after the flex-array.
> >
> > [1]
> > https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
> > ---
> > net/openvswitch/flow_table.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/openvswitch/flow_table.h b/net/openvswitch/flow_table.h
> > index 9e659db78c05..f524dc3e4862 100644
> > --- a/net/openvswitch/flow_table.h
> > +++ b/net/openvswitch/flow_table.h
> > @@ -48,7 +48,7 @@ struct mask_array {
> > int count, max;
> > struct mask_array_stats __percpu *masks_usage_stats;
> > u64 *masks_usage_zero_cntr;
> > - struct sw_flow_mask __rcu *masks[];
> > + struct sw_flow_mask __rcu *masks[] __counted_by(max);
> > };
>
> Yup, this looks correct to me. Thanks!
>
> Reviewed-by: Kees Cook
>
> --
> Kees Cook
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [Cluster-devel] [PATCH 00/12] drop unneeded newline
On Tue, 2 Jan 2018, Bart Van Assche wrote: > On Tue, 2018-01-02 at 15:00 +0100, Julia Lawall wrote: > > On Tue, 2 Jan 2018, Bob Peterson wrote: > > > - Original Message - > > > > - Original Message - > > > > > > > Still, the GFS2 and DLM code has a plethora of broken-up printk messages, > > > and I don't like the thought of re-combining them all. > > > > Actually, the point of the patch was to remove the unnecessary \n at the > > end of the string, because log_print will add another one. If you prefer > > to keep the string broken up, I can resend the patch in that form, but > > without the unnecessary \n. > > Please combine any user-visible strings into a single line for which the > unneeded newline is dropped since these strings are modified anyway by > your patch. That is what the submitted patch (2/12 specifically) did. julia ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [Cluster-devel] [PATCH 00/12] drop unneeded newline
On Tue, 2 Jan 2018, Bob Peterson wrote: > - Original Message - > | - Original Message - > | | Drop newline at the end of a message string when the printing function > adds > | | a newline. > | > | Hi Julia, > | > | NACK. > | > | As much as it's a pain when searching the source code for output strings, > | this patch set goes against the accepted Linux coding style document. See: > | > | > https://www.kernel.org/doc/html/v4.10/process/coding-style.html#breaking-long-lines-and-strings > | > | Regards, > | > | Bob Peterson > | > | > Hm. I guess I stand corrected. The document reads: > > "However, never break user-visible strings such as printk messages, because > that breaks the ability to grep for them." > > Still, the GFS2 and DLM code has a plethora of broken-up printk messages, > and I don't like the thought of re-combining them all. Actually, the point of the patch was to remove the unnecessary \n at the end of the string, because log_print will add another one. If you prefer to keep the string broken up, I can resend the patch in that form, but without the unnecessary \n. julia ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [Cluster-devel] [PATCH 00/12] drop unneeded newline
On Tue, 2 Jan 2018, Bob Peterson wrote: > - Original Message - > | Drop newline at the end of a message string when the printing function adds > | a newline. > > Hi Julia, > > NACK. > > As much as it's a pain when searching the source code for output strings, > this patch set goes against the accepted Linux coding style document. See: > > https://www.kernel.org/doc/html/v4.10/process/coding-style.html#breaking-long-lines-and-strings I don't think that's the case: "However, never break user-visible strings such as printk messages, because that breaks the ability to grep for them." julia > > Regards, > > Bob Peterson > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 05/12] openvswitch: drop unneeded newline
OVS_NLERR prints a newline at the end of the message string, so the
message string does not need to include a newline explicitly. Done
using Coccinelle.
Signed-off-by: Julia Lawall <julia.law...@lip6.fr>
---
net/openvswitch/conntrack.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index b27c5c6..62f36cc9 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -1266,14 +1266,14 @@ static int parse_nat(const struct nlattr *attr,
/* Do not allow flags if no type is given. */
if (info->range.flags) {
OVS_NLERR(log,
- "NAT flags may be given only when NAT range
(SRC or DST) is also specified.\n"
+ "NAT flags may be given only when NAT range
(SRC or DST) is also specified."
);
return -EINVAL;
}
info->nat = OVS_CT_NAT; /* NAT existing connections. */
} else if (!info->commit) {
OVS_NLERR(log,
- "NAT attributes may be specified only when CT COMMIT
flag is also specified.\n"
+ "NAT attributes may be specified only when CT COMMIT
flag is also specified."
);
return -EINVAL;
}
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
