On Wed, Aug 21, 2024 at 10:44 AM Kees Cook <k...@kernel.org> wrote:
>
> On Wed, Aug 21, 2024 at 03:27:56PM +0000, Qing Zhao wrote:
> > > On Aug 21, 2024, at 10:45, Martin Uecker <uec...@tugraz.at> wrote:
> > >
> > > Am Mittwoch, dem 21.08.2024 um 16:34 +0200 schrieb Martin Uecker:
> > >> Am Mittwoch, dem 21.08.2024 um 14:12 +0000 schrieb Qing Zhao:
> > >>
> > >>>
> > >>> Yes, I do feel that the approach __builtin_get_counted_by is not very
> > >>> good.
> > >>> Maybe it’s better to provide
> > >>> A. __builtin_set_counted_by
> > >>> or
> > >>> B. The unary operator __counted_by(PTR) to return a Lvalue, in this
> > >>> case,
> > >>> we need a __builtin_has_attribute first to check whether PTR has the
> > >>> counted_by attribute first.
> > >>
> > >> You could potentially do the same __counted_by and test for type void.
> > >>
> > >> _Generic(typeof(__counted_by(PTR)), void: (void)0, __counted_by(PTR) =
> > >> COUNT);
> > >
> > > But just doing A. also seems ok.
> >
> > I am fine with A. It’s easier to be used by the end users.
> >
> > The only potential problem with A is, the functionality of READing the
> > counted-by field is missing.
> > Is that okay? Kees?
>
> After seeing the utility of __builtin_get_counted_by() I realized that
> we really do want it for the ability to examine the _type_ of the
> counter member, otherwise we run the risk of assignment truncation. For
> example:
>
> struct flex {
> unsigned char counter;
> int array[] __attribute__((counted_by(counter)));
> } *p;
>
> count = 1000;
> ...
> __builtin_set_counted_by(p->array, count);
>
> What value would p->counter end up with? (I assume it would wrap around,
> which is bad). And there would be no way to catch it at run-time without
> a way to check the type. For example with __builtin_get_counted_by, we
> can do:
>
It might be check-able. SEMA would have to look at the arguments and
determine whether there's a size difference. Not the most elegant way
of doing things for sure. Using __builtin_get_counted_by() has its
return type changed to the counter's type, so it can go through
"normal" checking.
> if (__builtin_get_counted_by(p->array)) {
> size_t max_value =
> type_max(typeof(*__builtin_get_counted_by(p->array)));
> if (count > type_max)
> ...fail cleanly...
> *__builtin_get_counted_by(p->array) = count;
> }
>
> I don't strictly need to READ the value (but it seems nice). Currently I can
> already do a READ with something like this:
>
> size_t count = __builtin_dynamic_object_size(p->array, 1) / sizeof(*p->array);
>
> But I don't have a way to examine the counter _type_ without
> __builtin_get_counted_by, so I prefer it over __builtin_set_counted_by.
>
Another thing to point out, the documented type signature of
__builtin_get_counted_by() is more-or-less meaningless. It should be
adjusted during SEMA to a pointer to the count's type. It's only "void
*" if the 'counted_by' attribute.
-bw
