PATCH: configurable tiling in cwm(1)
Hello @tech,
with this diff https://marc.info/?l=openbsd-tech=149192690925713=2 the
behaviour of cwm changed so vtile and htile always use 50% of the screen.
This is a reasonable default but sometimes I want to have the master windows
be bigger.
Inspired by this patch https://marc.info/?l=openbsd-tech=154887686615696=2
I suggest the following diff.
This adds two new config options "htile percent" and "vtile percent" which
define how much of the screen the current windows should occupy as the master
window. If set to "0" then the old behaviour is restored where the current
vertical or horizontal size of the window defines the size of the master
window. If unset the current behaviour is unchaged that means the master
windows uses half of the screen size.
For example in .cwmrc:
htile 66
vtile 0
For window-htile the master window will occupy 66% of the screen high or for
window-vtile the current window width defines the screen width of the master
window.
Any thoughts?
Regards Uwe
Index: calmwm.h
===
RCS file: /cvs/xenocara/app/cwm/calmwm.h,v
retrieving revision 1.374
diff -u -p -r1.374 calmwm.h
--- calmwm.h24 Mar 2020 14:47:29 - 1.374
+++ calmwm.h27 Mar 2020 00:09:43 -
@@ -291,6 +291,8 @@ struct conf {
int bwidth;
int mamount;
int snapdist;
+ int htile;
+ int vtile;
struct gap gap;
char*color[CWM_COLOR_NITEMS];
char*font;
Index: client.c
===
RCS file: /cvs/xenocara/app/cwm/client.c,v
retrieving revision 1.262
diff -u -p -r1.262 client.c
--- client.c24 Mar 2020 14:47:29 - 1.262
+++ client.c27 Mar 2020 00:09:43 -
@@ -940,7 +940,8 @@ client_htile(struct client_ctx *cc)
cc->geom.x = area.x;
cc->geom.y = area.y;
cc->geom.w = area.w - (cc->bwidth * 2);
- cc->geom.h = (area.h - (cc->bwidth * 2)) / 2;
+ if (Conf.htile > 0)
+ cc->geom.h = ((area.h - (cc->bwidth * 2)) * Conf.htile) / 100;
client_resize(cc, 1);
client_ptr_warp(cc);
@@ -1007,7 +1008,8 @@ client_vtile(struct client_ctx *cc)
cc->flags &= ~CLIENT_VMAXIMIZED;
cc->geom.x = area.x;
cc->geom.y = area.y;
- cc->geom.w = (area.w - (cc->bwidth * 2)) / 2;
+ if (Conf.vtile > 0)
+ cc->geom.w = ((area.w - (cc->bwidth * 2)) * Conf.vtile) / 100;
cc->geom.h = area.h - (cc->bwidth * 2);
client_resize(cc, 1);
client_ptr_warp(cc);
Index: conf.c
===
RCS file: /cvs/xenocara/app/cwm/conf.c,v
retrieving revision 1.251
diff -u -p -r1.251 conf.c
--- conf.c 27 Feb 2020 14:56:39 - 1.251
+++ conf.c 27 Mar 2020 00:09:43 -
@@ -281,6 +281,8 @@ conf_init(struct conf *c)
c->stickygroups = 0;
c->bwidth = 1;
c->mamount = 1;
+ c->htile = 50;
+ c->vtile = 50;
c->snapdist = 0;
c->ngroups = 0;
c->nameqlen = 5;
Index: cwmrc.5
===
RCS file: /cvs/xenocara/app/cwm/cwmrc.5,v
retrieving revision 1.75
diff -u -p -r1.75 cwmrc.5
--- cwmrc.5 13 Mar 2020 20:50:07 - 1.75
+++ cwmrc.5 27 Mar 2020 00:09:43 -
@@ -183,6 +183,11 @@ This
can be used for applications such as
.Xr xclock 1 ,
where the user may wish to remain visible.
+.It Ic htile Ar percent
+The amount of the screen the master window should be resized to when
+.Ic window-htile
+is called. If set to 0 then the size is the vertical size of current current
+window.
.It Ic ignore Ar windowname
Ignore, and do not warp to, windows with the name
.Ar windowname
@@ -216,6 +221,11 @@ A special
keyword
.Dq all
can be used to unbind all buttons.
+.It Ic vtile Ar percent
+The amount of the screen the master window should be resized to when
+.Ic window-vtile
+is called. If set to 0 then the size is the horizontal size of the current
+window.
.It Ic wm Ar name path
Every
.Ar name
@@ -303,11 +313,15 @@ Vertically maximize current window (gap
Horizontally maximize current window (gap + border honored).
.It window-htile
Current window is placed at the top of the screen, maximized
-horizontally and resized to half of the vertical screen space.
+horizontally and resized to
+.Ar htile
+(default half) of the vertical screen space.
Other windows in its group share remaining screen space.
.It window-vtile
Current window is placed on the left of the screen, maximized vertically
-and resized to half of the horizontal screen space.
+and resized to
+.Ar vtile
+(default half) of the horizontal screen space.
Other windows in its group share remaining screen space.
.It window-move
Move current window.
Index: parse.y
[PATCH] gostr341001: support unwrapped private keys support
GOST private keys can be wrapped in OCTET STRING, INTEGER or come
unwrapped. Support the latter format.
Sponsored by ROSA Linux
Signed-off-by: Dmitry Baryshkov
---
src/lib/libcrypto/gost/gostr341001_ameth.c | 75 --
1 file changed, 70 insertions(+), 5 deletions(-)
diff --git a/src/lib/libcrypto/gost/gostr341001_ameth.c
b/src/lib/libcrypto/gost/gostr341001_ameth.c
index 0f816377dde1..70bd3357f184 100644
--- a/src/lib/libcrypto/gost/gostr341001_ameth.c
+++ b/src/lib/libcrypto/gost/gostr341001_ameth.c
@@ -437,6 +437,56 @@ priv_print_gost01(BIO *out, const EVP_PKEY *pkey, int
indent, ASN1_PCTX *pctx)
return pub_print_gost01(out, pkey, indent, pctx);
}
+static BIGNUM *unmask_priv_key(EVP_PKEY *pk,
+ const unsigned char *buf, int len, int num_masks)
+{
+ BIGNUM *pknum_masked = NULL, *q = NULL;
+ const GOST_KEY *key_ptr = pk->pkey.gost;
+ const EC_GROUP *group = GOST_KEY_get0_group(key_ptr);
+
+ pknum_masked = GOST_le2bn(buf, len, NULL);
+ if (!pknum_masked) {
+ GOSTerror(ERR_R_MALLOC_FAILURE);
+ return NULL;
+ }
+
+ if (num_masks > 0) {
+ /*
+* XXX Remove sign by gost94
+*/
+ const unsigned char *p = buf + num_masks * len;
+
+ q = BN_new();
+ if (!q) {
+ GOSTerror(ERR_R_MALLOC_FAILURE);
+ BN_free(pknum_masked);
+ pknum_masked = NULL;
+ goto end;
+ }
+ if (EC_GROUP_get_order(group, q, NULL) <= 0) {
+ GOSTerror(ERR_R_EC_LIB);
+ BN_free(pknum_masked);
+ pknum_masked = NULL;
+ goto end;
+ }
+
+ for (; p != buf; p -= len) {
+ BIGNUM *mask = GOST_le2bn(p, len, NULL);
+ BN_CTX *ctx = BN_CTX_new();
+
+ BN_mod_mul(pknum_masked, pknum_masked, mask, q, ctx);
+
+ BN_CTX_free(ctx);
+ BN_free(mask);
+ }
+ }
+
+end:
+ if (q)
+ BN_free(q);
+ return pknum_masked;
+}
+
static int
priv_decode_gost01(EVP_PKEY *pk, const PKCS8_PRIV_KEY_INFO *p8inf)
{
@@ -450,6 +500,7 @@ priv_decode_gost01(EVP_PKEY *pk, const PKCS8_PRIV_KEY_INFO
*p8inf)
GOST_KEY *ec;
int ptype = V_ASN1_UNDEF;
ASN1_STRING *pval = NULL;
+ int expected_key_len;
if (PKCS8_pkey_get0(_obj, _buf, _len, , p8inf) ==
0) {
GOSTerror(GOST_R_BAD_KEY_PARAMETERS_FORMAT);
@@ -467,29 +518,43 @@ priv_decode_gost01(EVP_PKEY *pk, const
PKCS8_PRIV_KEY_INFO *p8inf)
return 0;
}
p = pkey_buf;
- if (V_ASN1_OCTET_STRING == *p) {
+
+ expected_key_len = (pkey_bits_gost01(pk) + 7) / 8;
+ if (expected_key_len == 0) {
+ EVPerror(EVP_R_DECODE_ERROR);
+ return 0;
+ } else if (priv_len % expected_key_len == 0) {
+ /* Key is not wrapped but masked */
+ pk_num = unmask_priv_key(pk, pkey_buf, expected_key_len,
+ priv_len / expected_key_len - 1);
+ } else if (V_ASN1_OCTET_STRING == *p) {
/* New format - Little endian octet string */
ASN1_OCTET_STRING *s =
d2i_ASN1_OCTET_STRING(NULL, , priv_len);
if (s == NULL) {
- GOSTerror(EVP_R_DECODE_ERROR);
+ EVPerror(EVP_R_DECODE_ERROR);
ASN1_STRING_free(s);
return 0;
}
pk_num = GOST_le2bn(s->data, s->length, NULL);
ASN1_STRING_free(s);
- } else {
+ } else if (V_ASN1_INTEGER == *p) {
priv_key = d2i_ASN1_INTEGER(NULL, , priv_len);
- if (priv_key == NULL)
+ if (priv_key == NULL) {
+ EVPerror(EVP_R_DECODE_ERROR);
return 0;
+ }
ret = ((pk_num = ASN1_INTEGER_to_BN(priv_key, NULL)) != NULL);
ASN1_INTEGER_free(priv_key);
if (ret == 0) {
- GOSTerror(EVP_R_DECODE_ERROR);
+ EVPerror(EVP_R_DECODE_ERROR);
return 0;
}
+ } else {
+ EVPerror(EVP_R_DECODE_ERROR);
+ return 0;
}
ec = pk->pkey.gost;
--
2.25.1
[PATCH 2/2] gost: use ECerror to report EC errors
From: Dmitry Baryshkov
GOST code uses GOSTerror(EC_R_foo) to report several errors. Use
ECerror(EC_R_foo) instead to make error messages match error code.
Sponsored by ROSA Linux.
Signed-off-by: Dmitry Baryshkov
---
src/lib/libcrypto/gost/gostr341001_ameth.c | 2 +-
src/lib/libcrypto/gost/gostr341001_key.c | 14 +++---
src/lib/libcrypto/gost/gostr341001_pmeth.c | 2 +-
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/lib/libcrypto/gost/gostr341001_ameth.c
b/src/lib/libcrypto/gost/gostr341001_ameth.c
index be621d0185dd..28ed55e6992f 100644
--- a/src/lib/libcrypto/gost/gostr341001_ameth.c
+++ b/src/lib/libcrypto/gost/gostr341001_ameth.c
@@ -547,7 +547,7 @@ param_decode_gost01(EVP_PKEY *pkey, const unsigned char
**pder, int derlen)
}
group = EC_GROUP_new_by_curve_name(nid);
if (group == NULL) {
- GOSTerror(EC_R_EC_GROUP_NEW_BY_NAME_FAILURE);
+ ECerror(EC_R_EC_GROUP_NEW_BY_NAME_FAILURE);
GOST_KEY_free(ec);
return 0;
}
diff --git a/src/lib/libcrypto/gost/gostr341001_key.c
b/src/lib/libcrypto/gost/gostr341001_key.c
index 0af39f21bf33..74f8cab9d86c 100644
--- a/src/lib/libcrypto/gost/gostr341001_key.c
+++ b/src/lib/libcrypto/gost/gostr341001_key.c
@@ -121,7 +121,7 @@ GOST_KEY_check_key(const GOST_KEY *key)
return 0;
}
if (EC_POINT_is_at_infinity(key->group, key->pub_key) != 0) {
- GOSTerror(EC_R_POINT_AT_INFINITY);
+ ECerror(EC_R_POINT_AT_INFINITY);
goto err;
}
if ((ctx = BN_CTX_new()) == NULL)
@@ -131,14 +131,14 @@ GOST_KEY_check_key(const GOST_KEY *key)
/* testing whether the pub_key is on the elliptic curve */
if (EC_POINT_is_on_curve(key->group, key->pub_key, ctx) == 0) {
- GOSTerror(EC_R_POINT_IS_NOT_ON_CURVE);
+ ECerror(EC_R_POINT_IS_NOT_ON_CURVE);
goto err;
}
/* testing whether pub_key * order is the point at infinity */
if ((order = BN_new()) == NULL)
goto err;
if (EC_GROUP_get_order(key->group, order, ctx) == 0) {
- GOSTerror(EC_R_INVALID_GROUP_ORDER);
+ ECerror(EC_R_INVALID_GROUP_ORDER);
goto err;
}
if (EC_POINT_mul(key->group, point, NULL, key->pub_key, order,
@@ -147,7 +147,7 @@ GOST_KEY_check_key(const GOST_KEY *key)
goto err;
}
if (EC_POINT_is_at_infinity(key->group, point) == 0) {
- GOSTerror(EC_R_WRONG_ORDER);
+ ECerror(EC_R_WRONG_ORDER);
goto err;
}
/*
@@ -156,7 +156,7 @@ GOST_KEY_check_key(const GOST_KEY *key)
*/
if (key->priv_key != NULL) {
if (BN_cmp(key->priv_key, order) >= 0) {
- GOSTerror(EC_R_WRONG_ORDER);
+ ECerror(EC_R_WRONG_ORDER);
goto err;
}
if (EC_POINT_mul(key->group, point, key->priv_key, NULL, NULL,
@@ -165,7 +165,7 @@ GOST_KEY_check_key(const GOST_KEY *key)
goto err;
}
if (EC_POINT_cmp(key->group, point, key->pub_key, ctx) != 0) {
- GOSTerror(EC_R_INVALID_PRIVATE_KEY);
+ ECerror(EC_R_INVALID_PRIVATE_KEY);
goto err;
}
}
@@ -212,7 +212,7 @@ GOST_KEY_set_public_key_affine_coordinates(GOST_KEY *key,
BIGNUM *x, BIGNUM *y)
* out of range.
*/
if (BN_cmp(x, tx) != 0 || BN_cmp(y, ty) != 0) {
- GOSTerror(EC_R_COORDINATES_OUT_OF_RANGE);
+ ECerror(EC_R_COORDINATES_OUT_OF_RANGE);
goto err;
}
if (GOST_KEY_set_public_key(key, point) == 0)
diff --git a/src/lib/libcrypto/gost/gostr341001_pmeth.c
b/src/lib/libcrypto/gost/gostr341001_pmeth.c
index 0eb1d873deaf..0e0cae99e3fc 100644
--- a/src/lib/libcrypto/gost/gostr341001_pmeth.c
+++ b/src/lib/libcrypto/gost/gostr341001_pmeth.c
@@ -246,7 +246,7 @@ pkey_gost01_sign(EVP_PKEY_CTX *ctx, unsigned char *sig,
size_t *siglen,
*siglen = 2 * size;
return 1;
} else if (*siglen < 2 * size) {
- GOSTerror(EC_R_BUFFER_TOO_SMALL);
+ ECerror(EC_R_BUFFER_TOO_SMALL);
return 0;
}
if (tbs_len != 32 && tbs_len != 64) {
--
2.25.1
[PATCH 1/2] gost: add missing error reporting
From: Dmitry Baryshkov
Add few more error reports to help debugging.
Sponsored by ROSA Linux.
Signed-off-by: Dmitry Baryshkov
---
src/lib/libcrypto/gost/gostr341001_ameth.c | 20 +++-
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/src/lib/libcrypto/gost/gostr341001_ameth.c
b/src/lib/libcrypto/gost/gostr341001_ameth.c
index 16295996dce7..be621d0185dd 100644
--- a/src/lib/libcrypto/gost/gostr341001_ameth.c
+++ b/src/lib/libcrypto/gost/gostr341001_ameth.c
@@ -96,15 +96,19 @@ decode_gost01_algor_params(EVP_PKEY *pkey, const unsigned
char **p, int len)
ec = pkey->pkey.gost;
if (ec == NULL) {
ec = GOST_KEY_new();
- if (ec == NULL)
+ if (ec == NULL) {
+ GOSTerror(ERR_R_MALLOC_FAILURE);
return 0;
+ }
if (EVP_PKEY_assign_GOST(pkey, ec) == 0)
return 0;
}
group = EC_GROUP_new_by_curve_name(param_nid);
- if (group == NULL)
+ if (group == NULL) {
+ ECerror(EC_R_EC_GROUP_NEW_BY_NAME_FAILURE);
return 0;
+ }
EC_GROUP_set_asn1_flag(group, OPENSSL_EC_NAMED_CURVE);
if (GOST_KEY_set_group(ec, group) == 0) {
EC_GROUP_free(group);
@@ -207,8 +211,10 @@ pub_decode_gost01(EVP_PKEY *pk, X509_PUBKEY *pub)
return 0;
}
p = pval->data;
- if (decode_gost01_algor_params(pk, , pval->length) == 0)
+ if (decode_gost01_algor_params(pk, , pval->length) == 0) {
+ GOSTerror(GOST_R_BAD_KEY_PARAMETERS_FORMAT);
return 0;
+ }
octet = d2i_ASN1_OCTET_STRING(NULL, _buf, pub_len);
if (octet == NULL) {
@@ -407,8 +413,10 @@ priv_decode_gost01(EVP_PKEY *pk, const PKCS8_PRIV_KEY_INFO
*p8inf)
int ptype = V_ASN1_UNDEF;
ASN1_STRING *pval = NULL;
- if (PKCS8_pkey_get0(_obj, _buf, _len, , p8inf) == 0)
+ if (PKCS8_pkey_get0(_obj, _buf, _len, , p8inf) ==
0) {
+ GOSTerror(GOST_R_BAD_KEY_PARAMETERS_FORMAT);
return 0;
+ }
(void)EVP_PKEY_assign_GOST(pk, NULL);
X509_ALGOR_get0(NULL, , (const void **), palg);
if (ptype != V_ASN1_SEQUENCE) {
@@ -416,8 +424,10 @@ priv_decode_gost01(EVP_PKEY *pk, const PKCS8_PRIV_KEY_INFO
*p8inf)
return 0;
}
p = pval->data;
- if (decode_gost01_algor_params(pk, , pval->length) == 0)
+ if (decode_gost01_algor_params(pk, , pval->length) == 0) {
+ GOSTerror(GOST_R_BAD_KEY_PARAMETERS_FORMAT);
return 0;
+ }
p = pkey_buf;
if (V_ASN1_OCTET_STRING == *p) {
/* New format - Little endian octet string */
--
2.25.1
[PATCH] ec: add support for several more GOST curves
From: Dmitry Baryshkov
Add support for GOST curves defined by RFC 7836 and
draft-deremin-rfc4491-bis. Add aliases for 256-bit GOST curves (see
draft-smyshlyaev-tls12-gost-suites).
Sponsored by ROSA Linux.
Signed-off-by: Dmitry Baryshkov
---
src/lib/libcrypto/ec/ec_curve.c | 158 +-
src/lib/libcrypto/objects/obj_mac.num | 6 +
src/lib/libcrypto/objects/objects.txt | 10 +-
3 files changed, 168 insertions(+), 6 deletions(-)
diff --git a/src/lib/libcrypto/ec/ec_curve.c b/src/lib/libcrypto/ec/ec_curve.c
index e075b1ed3ea5..a1bc88ee2cc6 100644
--- a/src/lib/libcrypto/ec/ec_curve.c
+++ b/src/lib/libcrypto/ec/ec_curve.c
@@ -2900,11 +2900,101 @@ static const struct {
}
};
+static const struct {
+ EC_CURVE_DATA h;
+ unsigned char data[0 + 32 * 6];
+}
+ _EC_GOST_2012_256_TC26_A = {
+ {
+ NID_X9_62_prime_field, 0, 32, 1
+ },
+ { /* no seed */
+ 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
/* p */
+ 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
+ 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
+ 0xFD, 0x97,
+ 0xc2, 0x17, 0x3f, 0x15, 0x13, 0x98, 0x16, 0x73, 0xaf, 0x48,
/* a */
+ 0x92, 0xc2, 0x30, 0x35, 0xa2, 0x7c, 0xe2, 0x5e, 0x20, 0x13,
+ 0xbf, 0x95, 0xaa, 0x33, 0xb2, 0x2c, 0x65, 0x6f, 0x27, 0x7e,
+ 0x73, 0x35,
+ 0x29, 0x5f, 0x9b, 0xae, 0x74, 0x28, 0xed, 0x9c, 0xcc, 0x20,
/* b */
+ 0xe7, 0xc3, 0x59, 0xa9, 0xd4, 0x1a, 0x22, 0xfc, 0xcd, 0x91,
+ 0x08, 0xe1, 0x7b, 0xf7, 0xba, 0x93, 0x37, 0xa6, 0xf8, 0xae,
+ 0x95, 0x13,
+ 0x91, 0xe3, 0x84, 0x43, 0xa5, 0xe8, 0x2c, 0x0d, 0x88, 0x09,
/* x */
+ 0x23, 0x42, 0x57, 0x12, 0xb2, 0xbb, 0x65, 0x8b, 0x91, 0x96,
+ 0x93, 0x2e, 0x02, 0xc7, 0x8b, 0x25, 0x82, 0xfe, 0x74, 0x2d,
+ 0xaa, 0x28,
+ 0x32, 0x87, 0x94, 0x23, 0xab, 0x1a, 0x03, 0x75, 0x89, 0x57,
/* y */
+ 0x86, 0xc4, 0xbb, 0x46, 0xe9, 0x56, 0x5f, 0xde, 0x0b, 0x53,
+ 0x44, 0x76, 0x67, 0x40, 0xaf, 0x26, 0x8a, 0xdb, 0x32, 0x32,
+ 0x2e, 0x5c,
+ 0x40, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
/* order */
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0f, 0xd8, 0xcd, 0xdf,
+ 0xc8, 0x7b, 0x66, 0x35, 0xc1, 0x15, 0xaf, 0x55, 0x6c, 0x36,
+ 0x0c, 0x67,
+ }
+};
+
static const struct {
EC_CURVE_DATA h;
unsigned char data[0 + 64 * 6];
}
- _EC_GOST_2012_TC26_A = {
+ _EC_GOST_2012_512_Test = {
+ {
+ NID_X9_62_prime_field, 0, 64, 1
+ },
+ { /* no seed */
+ 0x45, 0x31, 0xac, 0xd1, 0xfe, 0x00, 0x23, 0xc7, 0x55, 0x0d,
/* p */
+ 0x26, 0x7b, 0x6b, 0x2f, 0xee, 0x80, 0x92, 0x2b, 0x14, 0xb2,
+ 0xff, 0xb9, 0x0f, 0x04, 0xd4, 0xeb, 0x7c, 0x09, 0xb5, 0xd2,
+ 0xd1, 0x5d, 0xf1, 0xd8, 0x52, 0x74, 0x1a, 0xf4, 0x70, 0x4a,
+ 0x04, 0x58, 0x04, 0x7e, 0x80, 0xe4, 0x54, 0x6d, 0x35, 0xb8,
+ 0x33, 0x6f, 0xac, 0x22, 0x4d, 0xd8, 0x16, 0x64, 0xbb, 0xf5,
+ 0x28, 0xbe, 0x63, 0x73,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
/* a */
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x07,
+ 0x1c, 0xff, 0x08, 0x06, 0xa3, 0x11, 0x16, 0xda, 0x29, 0xd8,
/* b */
+ 0xcf, 0xa5, 0x4e, 0x57, 0xeb, 0x74, 0x8b, 0xc5, 0xf3, 0x77,
+ 0xe4, 0x94, 0x00, 0xfd, 0xd7, 0x88, 0xb6, 0x49, 0xec, 0xa1,
+ 0xac, 0x43, 0x61, 0x83, 0x40, 0x13, 0xb2, 0xad, 0x73, 0x22,
+ 0x48, 0x0a, 0x89, 0xca, 0x58, 0xe0, 0xcf, 0x74, 0xbc, 0x9e,
+ 0x54, 0x0c, 0x2a, 0xdd, 0x68, 0x97, 0xfa, 0xd0, 0xa3, 0x08,
+ 0x4f, 0x30, 0x2a, 0xdc,
+ 0x24, 0xd1, 0x9c, 0xc6, 0x45, 0x72, 0xee, 0x30, 0xf3, 0x96,
/* x */
+ 0xbf, 0x6e, 0xbb, 0xfd, 0x7a, 0x6c, 0x52, 0x13, 0xb3, 0xb3,
+ 0xd7, 0x05, 0x7c, 0xc8, 0x25, 0xf9, 0x10, 0x93, 0xa6, 0x8c,
+ 0xd7, 0x62, 0xfd, 0x60, 0x61, 0x12, 0x62, 0xcd, 0x83, 0x8d,
+ 0xc6, 0xb6, 0x0a, 0xa7, 0xee, 0xe8, 0x04, 0xe2, 0x8b, 0xc8,
+ 0x49, 0x97, 0x7f, 0xac, 0x33, 0xb4, 0xb5, 0x30, 0xf1, 0xb1,
+ 0x20, 0x24, 0x8a, 0x9a,
+ 0x2b, 0xb3, 0x12, 0xa4, 0x3b, 0xd2, 0xce, 0x6e, 0x0d, 0x02,
/* y */
+
Re: Add missing #ifdefs to pppx_if_destroy()
On Thu, Mar 26, 2020 at 01:46:29PM +0100, Martin Pieuchot wrote:
> Does the diff below works for you? Are you ok with the direction? Any
> comment?
Diff works for me, Except you missed switch in the and of
pppx_add_session() and pipex_add_session().
Index: sys/net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.76
diff -u -p -r1.76 if_pppx.c
--- sys/net/if_pppx.c 20 Feb 2020 16:56:52 - 1.76
+++ sys/net/if_pppx.c 26 Mar 2020 13:21:32 -
@@ -675,12 +675,9 @@ pppx_add_session(struct pppx_dev *pxd, s
return (EINVAL);
break;
#endif
-#ifdef PIPEX_PPTP
+#if defined(PIPEX_PPTP) || defined(PIPEX_L2TP)
case PIPEX_PROTO_PPTP:
-#endif
-#ifdef PIPEX_L2TP
case PIPEX_PROTO_L2TP:
-#endif
switch (req->pr_peer_address.ss_family) {
case AF_INET:
if (req->pr_peer_address.ss_len != sizeof(struct
sockaddr_in))
@@ -701,6 +698,7 @@ pppx_add_session(struct pppx_dev *pxd, s
req->pr_local_address.ss_len)
return (EINVAL);
break;
+#endif /* defined(PIPEX_PPTP) || defined(PIPEX_L2TP) */
default:
return (EPROTONOSUPPORT);
}
@@ -854,6 +852,7 @@ pppx_add_session(struct pppx_dev *pxd, s
chain = PIPEX_ID_HASHTABLE(session->session_id);
LIST_INSERT_HEAD(chain, session, id_chain);
LIST_INSERT_HEAD(_session_list, session, session_list);
+#if defined(PIPEX_PPTP) || defined(PIPEX_L2TP)
switch (req->pr_protocol) {
case PIPEX_PROTO_PPTP:
case PIPEX_PROTO_L2TP:
@@ -862,6 +861,7 @@ pppx_add_session(struct pppx_dev *pxd, s
LIST_INSERT_HEAD(chain, session, peer_addr_chain);
break;
}
+#endif
/* if first session is added, start timer */
if (LIST_NEXT(session, session_list) == NULL)
@@ -967,13 +967,14 @@ pppx_if_destroy(struct pppx_dev *pxd, st
LIST_REMOVE(session, id_chain);
LIST_REMOVE(session, session_list);
+#if defined(PIPEX_PPTP) || defined(PIPEX_L2TP)
switch (session->protocol) {
case PIPEX_PROTO_PPTP:
case PIPEX_PROTO_L2TP:
- LIST_REMOVE((struct pipex_session *)session,
- peer_addr_chain);
+ LIST_REMOVE(session, peer_addr_chain);
break;
}
+#endif
/* if final session is destroyed, stop timer */
if (LIST_EMPTY(_session_list))
Index: sys/net/pipex.c
===
RCS file: /cvs/src/sys/net/pipex.c,v
retrieving revision 1.108
diff -u -p -r1.108 pipex.c
--- sys/net/pipex.c 25 Mar 2020 11:39:58 - 1.108
+++ sys/net/pipex.c 26 Mar 2020 13:21:33 -
@@ -283,12 +283,8 @@ pipex_add_session(struct pipex_session_r
break;
#endif
#if defined(PIPEX_L2TP) || defined(PIPEX_PPTP)
-#ifdef PIPEX_PPTP
case PIPEX_PROTO_PPTP:
-#endif
-#ifdef PIPEX_L2TP
case PIPEX_PROTO_L2TP:
-#endif
switch (req->pr_peer_address.ss_family) {
case AF_INET:
if (req->pr_peer_address.ss_len !=
@@ -311,7 +307,7 @@ pipex_add_session(struct pipex_session_r
req->pr_local_address.ss_len)
return (EINVAL);
break;
-#endif
+#endif /* defined(PIPEX_PPTP) || defined(PIPEX_L2TP) */
default:
return (EPROTONOSUPPORT);
}
@@ -450,6 +446,7 @@ pipex_add_session(struct pipex_session_r
chain = PIPEX_ID_HASHTABLE(session->session_id);
LIST_INSERT_HEAD(chain, session, id_chain);
LIST_INSERT_HEAD(_session_list, session, session_list);
+#if defined(PIPEX_PPTP) || defined(PIPEX_L2TP)
switch (req->pr_protocol) {
case PIPEX_PROTO_PPTP:
case PIPEX_PROTO_L2TP:
@@ -457,6 +454,7 @@ pipex_add_session(struct pipex_session_r
pipex_sockaddr_hash_key(>peer.sa));
LIST_INSERT_HEAD(chain, session, peer_addr_chain);
}
+#endif
/* if first session is added, start timer */
if (LIST_NEXT(session, session_list) == NULL)
@@ -581,16 +579,15 @@ pipex_destroy_session(struct pipex_sessi
LIST_REMOVE(session, id_chain);
LIST_REMOVE(session, session_list);
-#ifdef PIPEX_PPTP
- if (session->protocol == PIPEX_PROTO_PPTP) {
- LIST_REMOVE(session, peer_addr_chain);
- }
-#endif
-#ifdef PIPEX_L2TP
- if (session->protocol == PIPEX_PROTO_L2TP) {
+#if defined(PIPEX_PPTP) || defined(PIPEX_L2TP)
+ switch (session->protocol) {
+ case PIPEX_PROTO_PPTP:
+ case PIPEX_PROTO_L2TP:
LIST_REMOVE(session, peer_addr_chain);
+ break;
}
#endif
+
/* if final session is destroyed, stop timer */
if (LIST_EMPTY(_session_list))
Re: Add missing #ifdefs to pppx_if_destroy()
On 26/03/20(Thu) 14:41, Vitaliy Makkoveev wrote:
> On Thu, Mar 26, 2020 at 11:56:27AM +0100, Martin Pieuchot wrote:
> > On 26/03/20(Thu) 13:34, Vitaliy Makkoveev wrote:
> > > Add missing #ifdefs to pppx_if_destroy() as it done in
> > > pipex_destroy_session(). Also remove unnecessary cast.
> >
> > What's the point of such #ifdef? I understand the current code is not
> > coherent, but does this reduce the binary size? For a case in a switch?
> > Because it clearly complicates the understanding of the code.
> Code coherency is the goal.
> > So maybe having #if defined(PIPEX_PPTP) || defined(PIPEX_L2TP) there as
> > well as in pppx_add_session() and pipex_destroy_session() is the way to
> > go.
> >
> > But the underlying question would it make sense to have pppx_if_destroy()
> > and pipex_destroy_session() call the same function to clear sessions?
> >
> > The same could be add for pipex_add_session() and pppx_add_session().
> My next goal.
Great!
> I updated this diff with '#if defined()...' and for
> pipex_destroy_session() too.
Here's what I meant. To remove the individual #ifdef. This is just
compiled as I don't have a setup to test this change right now.
What's interesting in that change is that `peer_addr_chain' appears to
be only used by PPTP and L2TP. Is it so? If that the case what does
that implies about data structure organization?
Does the diff below works for you? Are you ok with the direction? Any
comment?
Index: net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.76
diff -u -p -r1.76 if_pppx.c
--- net/if_pppx.c 20 Feb 2020 16:56:52 - 1.76
+++ net/if_pppx.c 26 Mar 2020 12:38:53 -
@@ -675,12 +675,9 @@ pppx_add_session(struct pppx_dev *pxd, s
return (EINVAL);
break;
#endif
-#ifdef PIPEX_PPTP
+#if defined(PIPEX_PPTP) || defined(PIPEX_L2TP)
case PIPEX_PROTO_PPTP:
-#endif
-#ifdef PIPEX_L2TP
case PIPEX_PROTO_L2TP:
-#endif
switch (req->pr_peer_address.ss_family) {
case AF_INET:
if (req->pr_peer_address.ss_len != sizeof(struct
sockaddr_in))
@@ -701,6 +698,7 @@ pppx_add_session(struct pppx_dev *pxd, s
req->pr_local_address.ss_len)
return (EINVAL);
break;
+#endif /* defined(PIPEX_PPTP) || defined(PIPEX_L2TP) */
default:
return (EPROTONOSUPPORT);
}
@@ -967,13 +965,14 @@ pppx_if_destroy(struct pppx_dev *pxd, st
LIST_REMOVE(session, id_chain);
LIST_REMOVE(session, session_list);
+#if defined(PIPEX_PPTP) || defined(PIPEX_L2TP)
switch (session->protocol) {
case PIPEX_PROTO_PPTP:
case PIPEX_PROTO_L2TP:
- LIST_REMOVE((struct pipex_session *)session,
- peer_addr_chain);
+ LIST_REMOVE(session, peer_addr_chain);
break;
}
+#endif
/* if final session is destroyed, stop timer */
if (LIST_EMPTY(_session_list))
Index: net/pipex.c
===
RCS file: /cvs/src/sys/net/pipex.c,v
retrieving revision 1.108
diff -u -p -r1.108 pipex.c
--- net/pipex.c 25 Mar 2020 11:39:58 - 1.108
+++ net/pipex.c 26 Mar 2020 12:41:09 -
@@ -283,12 +283,8 @@ pipex_add_session(struct pipex_session_r
break;
#endif
#if defined(PIPEX_L2TP) || defined(PIPEX_PPTP)
-#ifdef PIPEX_PPTP
case PIPEX_PROTO_PPTP:
-#endif
-#ifdef PIPEX_L2TP
case PIPEX_PROTO_L2TP:
-#endif
switch (req->pr_peer_address.ss_family) {
case AF_INET:
if (req->pr_peer_address.ss_len !=
@@ -311,7 +307,7 @@ pipex_add_session(struct pipex_session_r
req->pr_local_address.ss_len)
return (EINVAL);
break;
-#endif
+#endif /* defined(PIPEX_PPTP) || defined(PIPEX_L2TP) */
default:
return (EPROTONOSUPPORT);
}
@@ -581,16 +577,15 @@ pipex_destroy_session(struct pipex_sessi
LIST_REMOVE(session, id_chain);
LIST_REMOVE(session, session_list);
-#ifdef PIPEX_PPTP
- if (session->protocol == PIPEX_PROTO_PPTP) {
- LIST_REMOVE(session, peer_addr_chain);
- }
-#endif
-#ifdef PIPEX_L2TP
- if (session->protocol == PIPEX_PROTO_L2TP) {
+#if defined(PIPEX_PPTP) || defined(PIPEX_L2TP)
+ switch (session->protocol) {
+ case PIPEX_PROTO_PPTP:
+ case PIPEX_PROTO_L2TP:
LIST_REMOVE(session, peer_addr_chain);
+ break;
}
#endif
+
/* if final session is destroyed, stop timer */
if (LIST_EMPTY(_session_list))
pipex_timer_stop();
Re: Add missing #ifdefs to pppx_if_destroy()
On Thu, Mar 26, 2020 at 11:56:27AM +0100, Martin Pieuchot wrote:
> On 26/03/20(Thu) 13:34, Vitaliy Makkoveev wrote:
> > Add missing #ifdefs to pppx_if_destroy() as it done in
> > pipex_destroy_session(). Also remove unnecessary cast.
>
> What's the point of such #ifdef? I understand the current code is not
> coherent, but does this reduce the binary size? For a case in a switch?
> Because it clearly complicates the understanding of the code.
Code coherency is the goal.
> So maybe having #if defined(PIPEX_PPTP) || defined(PIPEX_L2TP) there as
> well as in pppx_add_session() and pipex_destroy_session() is the way to
> go.
>
> But the underlying question would it make sense to have pppx_if_destroy()
> and pipex_destroy_session() call the same function to clear sessions?
>
> The same could be add for pipex_add_session() and pppx_add_session().
My next goal.
I updated this diff with '#if defined()...' and for
pipex_destroy_session() too.
Index: sys/net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.76
diff -u -p -r1.76 if_pppx.c
--- sys/net/if_pppx.c 20 Feb 2020 16:56:52 - 1.76
+++ sys/net/if_pppx.c 26 Mar 2020 11:27:07 -
@@ -967,13 +967,18 @@ pppx_if_destroy(struct pppx_dev *pxd, st
LIST_REMOVE(session, id_chain);
LIST_REMOVE(session, session_list);
+#if defined(PIPEX_PPTP) || defined(PIPEX_L2TP)
switch (session->protocol) {
+#ifdef PIPEX_PPTP
case PIPEX_PROTO_PPTP:
+#endif
+#ifdef PIPEX_L2TP
case PIPEX_PROTO_L2TP:
- LIST_REMOVE((struct pipex_session *)session,
- peer_addr_chain);
+#endif
+ LIST_REMOVE(session, peer_addr_chain);
break;
}
+#endif
/* if final session is destroyed, stop timer */
if (LIST_EMPTY(_session_list))
Index: sys/net/pipex.c
===
RCS file: /cvs/src/sys/net/pipex.c,v
retrieving revision 1.108
diff -u -p -r1.108 pipex.c
--- sys/net/pipex.c 25 Mar 2020 11:39:58 - 1.108
+++ sys/net/pipex.c 26 Mar 2020 11:27:08 -
@@ -581,16 +581,19 @@ pipex_destroy_session(struct pipex_sessi
LIST_REMOVE(session, id_chain);
LIST_REMOVE(session, session_list);
+#if defined(PIPEX_PPTP) || defined(PIPEX_L2TP)
+ switch (session->protocol) {
#ifdef PIPEX_PPTP
- if (session->protocol == PIPEX_PROTO_PPTP) {
- LIST_REMOVE(session, peer_addr_chain);
- }
+ case PIPEX_PROTO_PPTP:
#endif
#ifdef PIPEX_L2TP
- if (session->protocol == PIPEX_PROTO_L2TP) {
+ case PIPEX_PROTO_L2TP:
+#endif
LIST_REMOVE(session, peer_addr_chain);
+ break;
}
#endif
+
/* if final session is destroyed, stop timer */
if (LIST_EMPTY(_session_list))
pipex_timer_stop();
MSI-X for ix(4) again
Previous diff had a bug in the interrupt handler refactoring that showed
up on i386. The version below has been tested on i386 and doesn't seem
to cause any regression.
Here's the original explanation, diff below has msix toggle to "off":
> Diff below allows ix(4) to establish two MSI-X handlers. Multiqueue
> support is intentionally not included in this diff.
>
> One handler is for the single queue (index ":0") and one for the
> link interrupt:
>
> # vmstat -i |grep ix0
> irq114/ix0:0 73178178 3758
> irq115/ix0 20
>
> A per-driver toggle allows to switch MSI-X support on/off. My plan is
> to commit with the switch "off" for the moment. Switching it to "on"
> should be better done in the beginning of a release cycle. However it
> is "on" in the diff below for testing purposes.
I appreciate more tests and oks :)
Index: dev/pci/if_ix.c
===
RCS file: /cvs/src/sys/dev/pci/if_ix.c,v
retrieving revision 1.163
diff -u -p -r1.163 if_ix.c
--- dev/pci/if_ix.c 25 Mar 2020 17:20:46 - 1.163
+++ dev/pci/if_ix.c 26 Mar 2020 10:37:41 -
@@ -114,6 +114,8 @@ int ixgbe_media_change(struct ifnet *);
void ixgbe_identify_hardware(struct ix_softc *);
intixgbe_allocate_pci_resources(struct ix_softc *);
intixgbe_allocate_legacy(struct ix_softc *);
+intixgbe_allocate_msix(struct ix_softc *);
+intixgbe_setup_msix(struct ix_softc *);
intixgbe_allocate_queues(struct ix_softc *);
void ixgbe_free_pci_resources(struct ix_softc *);
void ixgbe_local_timer(void *);
@@ -140,6 +142,7 @@ voidixgbe_initialize_rss_mapping(struct
intixgbe_rxfill(struct rx_ring *);
void ixgbe_rxrefill(void *);
+intixgbe_intr(struct ix_softc *sc);
void ixgbe_enable_intr(struct ix_softc *);
void ixgbe_disable_intr(struct ix_softc *);
void ixgbe_update_stats_counters(struct ix_softc *);
@@ -172,11 +175,16 @@ void ixgbe_handle_msf(struct ix_softc *)
void ixgbe_handle_phy(struct ix_softc *);
/* Legacy (single vector interrupt handler */
-intixgbe_intr(void *);
+intixgbe_legacy_intr(void *);
void ixgbe_enable_queue(struct ix_softc *, uint32_t);
+void ixgbe_enable_queues(struct ix_softc *);
void ixgbe_disable_queue(struct ix_softc *, uint32_t);
void ixgbe_rearm_queue(struct ix_softc *, uint32_t);
+/* MSI-X (multiple vectors interrupt handlers) */
+intixgbe_link_intr(void *);
+intixgbe_queue_intr(void *);
+
/*
* OpenBSD Device Interface Entry Points
*/
@@ -190,6 +198,7 @@ struct cfattach ix_ca = {
};
int ixgbe_smart_speed = ixgbe_smart_speed_on;
+int ixgbe_enable_msix = 0;
/*
* Device identification routine
@@ -292,7 +301,10 @@ ixgbe_attach(struct device *parent, stru
bcopy(sc->hw.mac.addr, sc->arpcom.ac_enaddr,
IXGBE_ETH_LENGTH_OF_ADDRESS);
- error = ixgbe_allocate_legacy(sc);
+ if (sc->msix > 1)
+ error = ixgbe_allocate_msix(sc);
+ else
+ error = ixgbe_allocate_legacy(sc);
if (error)
goto err_late;
@@ -524,6 +536,7 @@ ixgbe_ioctl(struct ifnet * ifp, u_long c
ixgbe_disable_intr(sc);
ixgbe_iff(sc);
ixgbe_enable_intr(sc);
+ ixgbe_enable_queues(sc);
}
error = 0;
}
@@ -819,6 +832,9 @@ ixgbe_init(void *arg)
itr |= IXGBE_EITR_LLI_MOD | IXGBE_EITR_CNT_WDIS;
IXGBE_WRITE_REG(>hw, IXGBE_EITR(0), itr);
+ /* Set moderation on the Link interrupt */
+ IXGBE_WRITE_REG(>hw, IXGBE_EITR(sc->linkvec), IXGBE_LINK_ITR);
+
/* Enable power to the phy */
if (sc->hw.phy.ops.set_phy_power)
sc->hw.phy.ops.set_phy_power(>hw, TRUE);
@@ -834,6 +850,7 @@ ixgbe_init(void *arg)
/* And now turn on interrupts */
ixgbe_enable_intr(sc);
+ ixgbe_enable_queues(sc);
/* Now inform the stack we're ready */
ifp->if_flags |= IFF_RUNNING;
@@ -964,6 +981,16 @@ ixgbe_enable_queue(struct ix_softc *sc,
}
void
+ixgbe_enable_queues(struct ix_softc *sc)
+{
+ struct ix_queue *que;
+ int i;
+
+ for (i = 0, que = sc->queues; i < sc->num_queues; i++, que++)
+ ixgbe_enable_queue(sc, que->msix);
+}
+
+void
ixgbe_disable_queue(struct ix_softc *sc, uint32_t vector)
{
uint64_t queue = 1ULL << vector;
@@ -982,6 +1009,41 @@ ixgbe_disable_queue(struct ix_softc *sc,
}
}
+/*
+ * MSIX Interrupt Handlers
+ */
+int
+ixgbe_link_intr(void *vsc)
+{
+ struct ix_softc *sc = (struct ix_softc *)vsc;
+
+ return ixgbe_intr(sc);
+}
+
+int
+ixgbe_queue_intr(void *vque)
+{
+
Re: Add missing #ifdefs to pppx_if_destroy()
On 26/03/20(Thu) 13:34, Vitaliy Makkoveev wrote:
> Add missing #ifdefs to pppx_if_destroy() as it done in
> pipex_destroy_session(). Also remove unnecessary cast.
What's the point of such #ifdef? I understand the current code is not
coherent, but does this reduce the binary size? For a case in a switch?
Because it clearly complicates the understanding of the code.
If one is going to remove support for any of the two, grepping for
PIPEX_PROTO_* will be necessary.
So maybe having #if defined(PIPEX_PPTP) || defined(PIPEX_L2TP) there as
well as in pppx_add_session() and pipex_destroy_session() is the way to
go.
But the underlying question would it make sense to have pppx_if_destroy()
and pipex_destroy_session() call the same function to clear sessions?
The same could be add for pipex_add_session() and pppx_add_session().
Any cleanup here is welcome, building understanding of the data
structures used by those pseudo-drivers is the way to get they out of
the KERNEL_LOCK(). If you're curious the entry point is pipexintr().
Getting that out of KERNEL_LOCK() will at least improve latency of the
system and is a required step to improve performances.
> Index: sys/net/if_pppx.c
> ===
> RCS file: /cvs/src/sys/net/if_pppx.c,v
> retrieving revision 1.76
> diff -u -p -r1.76 if_pppx.c
> --- sys/net/if_pppx.c 20 Feb 2020 16:56:52 - 1.76
> +++ sys/net/if_pppx.c 26 Mar 2020 10:07:26 -
> @@ -967,13 +967,14 @@ pppx_if_destroy(struct pppx_dev *pxd, st
>
> LIST_REMOVE(session, id_chain);
> LIST_REMOVE(session, session_list);
> - switch (session->protocol) {
> - case PIPEX_PROTO_PPTP:
> - case PIPEX_PROTO_L2TP:
> - LIST_REMOVE((struct pipex_session *)session,
> - peer_addr_chain);
> - break;
> - }
> +#ifdef PIPEX_PPTP
> + if (session->protocol == PIPEX_PROTO_PPTP)
> + LIST_REMOVE(session, peer_addr_chain);
> +#endif
> +#ifdef PIPEX_L2TP
> + if (session->protocol == PIPEX_PROTO_L2TP)
> + LIST_REMOVE(session, peer_addr_chain);
> +#endif
>
> /* if final session is destroyed, stop timer */
> if (LIST_EMPTY(_session_list))
>
Add missing #ifdefs to pppx_if_destroy()
Add missing #ifdefs to pppx_if_destroy() as it done in
pipex_destroy_session(). Also remove unnecessary cast.
Index: sys/net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.76
diff -u -p -r1.76 if_pppx.c
--- sys/net/if_pppx.c 20 Feb 2020 16:56:52 - 1.76
+++ sys/net/if_pppx.c 26 Mar 2020 10:07:26 -
@@ -967,13 +967,14 @@ pppx_if_destroy(struct pppx_dev *pxd, st
LIST_REMOVE(session, id_chain);
LIST_REMOVE(session, session_list);
- switch (session->protocol) {
- case PIPEX_PROTO_PPTP:
- case PIPEX_PROTO_L2TP:
- LIST_REMOVE((struct pipex_session *)session,
- peer_addr_chain);
- break;
- }
+#ifdef PIPEX_PPTP
+ if (session->protocol == PIPEX_PROTO_PPTP)
+ LIST_REMOVE(session, peer_addr_chain);
+#endif
+#ifdef PIPEX_L2TP
+ if (session->protocol == PIPEX_PROTO_L2TP)
+ LIST_REMOVE(session, peer_addr_chain);
+#endif
/* if final session is destroyed, stop timer */
if (LIST_EMPTY(_session_list))
Re: softraid_raid5: possible NULL dereference
> Date: Thu, 26 Mar 2020 01:03:26 +0100 > From: Tobias Heider > > sr_block_get() returns dma_alloc(length, PR_NOWAIT | PR_ZERO) which may be > NULL if the memory pool is depleted. > The result is used as 'dst' argument to memcpy() in the following call to > sr_raid5_regenerate(), resulting in a possible NULL dereference. > > ok? > > Index: softraid_raid5.c > === > RCS file: /mount/openbsd/cvs/src/sys/dev/softraid_raid5.c,v > retrieving revision 1.29 > diff -u -p -r1.29 softraid_raid5.c > --- softraid_raid5.c 8 Aug 2019 02:19:55 - 1.29 > +++ softraid_raid5.c 25 Mar 2020 23:54:25 - > @@ -818,7 +818,8 @@ sr_raid5_rebuild(struct sr_discipline *s > wu_w = sr_scsi_wu_get(sd, 0); > wu_r = sr_scsi_wu_get(sd, 0); > > - xorbuf = sr_block_get(sd, strip_size); > + if ((xorbuf = sr_block_get(sd, strip_size)) == NULL) > + goto bad; > if (sr_raid5_regenerate(wu_r, rebuild_chunk, chunk_lba, > strip_size, xorbuf)) > goto bad; As a matter of style, it is better to avoid assignments in if-statements.
Re: softraid_raid5: possible NULL dereference
On Thu, Mar 26, 2020 at 01:03:26AM +0100, Tobias Heider wrote: > sr_block_get() returns dma_alloc(length, PR_NOWAIT | PR_ZERO) which may be > NULL if the memory pool is depleted. > The result is used as 'dst' argument to memcpy() in the following call to > sr_raid5_regenerate(), resulting in a possible NULL dereference. > > ok? > > Index: softraid_raid5.c > === > RCS file: /mount/openbsd/cvs/src/sys/dev/softraid_raid5.c,v > retrieving revision 1.29 > diff -u -p -r1.29 softraid_raid5.c > --- softraid_raid5.c 8 Aug 2019 02:19:55 - 1.29 > +++ softraid_raid5.c 25 Mar 2020 23:54:25 - > @@ -818,7 +818,8 @@ sr_raid5_rebuild(struct sr_discipline *s > wu_w = sr_scsi_wu_get(sd, 0); > wu_r = sr_scsi_wu_get(sd, 0); > > - xorbuf = sr_block_get(sd, strip_size); > + if ((xorbuf = sr_block_get(sd, strip_size)) == NULL) > + goto bad; > if (sr_raid5_regenerate(wu_r, rebuild_chunk, chunk_lba, > strip_size, xorbuf)) > goto bad; > Obvious fix. ok stsp@ Same problem is present in sr_raid5_scrub() which is currently in #if 0.
Re: umidi: missing NULL checks
On Mon, Mar 23, 2020 at 10:51:06PM +0100, Tobias Heider wrote:
> In alloc_all_jacks() the variables 'sc_in_jacks' and 'sc_out_checks'
> are set to NULL if 'sc_in_num_jacks' and 'sc_out_num_jacks' are 0.
> Further down both are dereferenced unconditionally. I added explicit NULL
> checks where I think they belong.
> I think 'sc_in_ep' and 'sc_out_ep' can also be NULL, but I am not
> entirely sure about those.
>
> Objections or ok?
>
sc_{out,in}_jacks is used only if sc_out_num_jacks > 0, i.e. if
there's at least one iteration. But if sc_out_num_jacks > 0, then
sc_{out,in}_jacks is properly initialized by this chunk:
sc->sc_out_jacks =
sc->sc_out_num_jacks ? sc->sc_jacks : NULL;
sc->sc_in_jacks =
sc->sc_in_num_jacks ? sc->sc_jacks + sc->sc_out_num_jacks : NULL;
AFAICS, the code is correct; but the style looks misleading to me.
> Index: dev/usb/umidi.c
> ===
> RCS file: /mount/openbsd/cvs/src/sys/dev/usb/umidi.c,v
> retrieving revision 1.53
> diff -u -p -r1.53 umidi.c
> --- dev/usb/umidi.c 16 Mar 2020 16:12:43 - 1.53
> +++ dev/usb/umidi.c 23 Mar 2020 21:50:49 -
> @@ -724,50 +724,58 @@ alloc_all_jacks(struct umidi_softc *sc)
> sc->sc_in_jacks =
> sc->sc_in_num_jacks ? sc->sc_jacks+sc->sc_out_num_jacks : NULL;
>
> - jack = >sc_out_jacks[0];
> - for (i=0; isc_out_num_jacks; i++) {
> - jack->opened = 0;
> - jack->binded = 0;
> - jack->arg = NULL;
> - jack->u.out.intr = NULL;
> - jack->intr = 0;
> - jack->cable_number = i;
> - jack++;
> + if (sc->sc_out_jacks) {
> + jack = >sc_out_jacks[0];
> + for (i=0; isc_out_num_jacks; i++) {
> + jack->opened = 0;
> + jack->binded = 0;
> + jack->arg = NULL;
> + jack->u.out.intr = NULL;
> + jack->intr = 0;
> + jack->cable_number = i;
> + jack++;
> + }
> }
> - jack = >sc_in_jacks[0];
> - for (i=0; isc_in_num_jacks; i++) {
> - jack->opened = 0;
> - jack->binded = 0;
> - jack->arg = NULL;
> - jack->u.in.intr = NULL;
> - jack->cable_number = i;
> - jack++;
> + if (sc->sc_in_jacks) {
> + jack = >sc_in_jacks[0];
> + for (i=0; isc_in_num_jacks; i++) {
> + jack->opened = 0;
> + jack->binded = 0;
> + jack->arg = NULL;
> + jack->u.in.intr = NULL;
> + jack->cable_number = i;
> + jack++;
> + }
> }
>
> /* assign each jacks to each endpoints */
> - jack = >sc_out_jacks[0];
> - ep = >sc_out_ep[0];
> - for (i=0; isc_out_num_endpoints; i++) {
> - rjack = >jacks[0];
> - for (j=0; jnum_jacks; j++) {
> - *rjack = jack;
> - jack->endpoint = ep;
> - jack++;
> - rjack++;
> + if (sc->sc_out_jacks && sc->sc_out_ep) {
> + jack = >sc_out_jacks[0];
> + ep = >sc_out_ep[0];
> + for (i=0; isc_out_num_endpoints; i++) {
> + rjack = >jacks[0];
> + for (j=0; jnum_jacks; j++) {
> + *rjack = jack;
> + jack->endpoint = ep;
> + jack++;
> + rjack++;
> + }
> + ep++;
> }
> - ep++;
> }
> - jack = >sc_in_jacks[0];
> - ep = >sc_in_ep[0];
> - for (i=0; isc_in_num_endpoints; i++) {
> - rjack = >jacks[0];
> - for (j=0; jnum_jacks; j++) {
> - *rjack = jack;
> - jack->endpoint = ep;
> - jack++;
> - rjack++;
> + if (sc->sc_in_jacks && sc->sc_in_ep) {
> + jack = >sc_in_jacks[0];
> + ep = >sc_in_ep[0];
> + for (i=0; isc_in_num_endpoints; i++) {
> + rjack = >jacks[0];
> + for (j=0; jnum_jacks; j++) {
> + *rjack = jack;
> + jack->endpoint = ep;
> + jack++;
> + rjack++;
> + }
> + ep++;
> }
> - ep++;
> }
>
> return USBD_NORMAL_COMPLETION;
>
