Ignat Korchagin <[email protected]> wrote:
> > + case OID_id_rsassa_pss:
> > + goto rsassa_pss;
> ...
> > +rsassa_pss:
> > + if (!ctx->algo_params || !ctx->algo_params_size) {
> > + pr_debug("RSASSA-PSS sig algo without parameters\n");
> > + return -EBADMSG;
> > + }
> > +
> > + err = rsassa_parse_sig_params(sig, ctx->algo_params,
> > ctx->algo_params_size);
> > + if (err < 0)
> > + return err;
> > +
> > + sig->pkey_algo = "rsa";
> > + sig->encoding = "emsa-pss";
> > + goto out;
> > }
>
> I really don't like this. Is it possible to factor this out to a
> separate function and just call here? Should the factored function
> even be part of the implementation somehow?
I'll move the check into rsassa_parse_sig_params() and then move the remaining
code back into the switch case. That also means that x509_note_sig_algo()
doesn't need the check either. It does change the pr_fmt value seen by the
pr_debug(), but that's probably fine.
> > ctx->last_oid = look_up_OID(value, vlen);
> > if (ctx->last_oid == OID__NR) {
> > - char buffer[50];
> > + char buffer[56];
> > sprint_oid(value, vlen, buffer, sizeof(buffer));
>
> I've seen this elsewhere in the crypto code (namely in ECC) but is it
> generally a good idea to declare long buffers on the stack?
It's not all that long (7 words on a 64-bit machine - similarish to a function
call), and the output of sprint_oid() is limited to it.
David
