On Wed, Aug 21, 2024 at 2:54 PM Bill Wendling <isanb...@gmail.com> wrote:
>
> 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.
>
Should read: "if the ' counted_by' attribute doesn't exist."
> -bw
