Put data to skb, decrypt with lib80211_crypt_wep, and place back to tx buffer.
Signed-off-by: Ivan Safonov <[email protected]>
---
drivers/staging/rtl8188eu/core/rtw_security.c | 72 ++++++++++++++++-----------
1 file changed, 43 insertions(+), 29 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_security.c b/drivers/staging/rtl8188eu/core/rtw_security.c
index bfe0b217e679..80d7569a3108 100644
--- a/drivers/staging/rtl8188eu/core/rtw_security.c
+++ b/drivers/staging/rtl8188eu/core/rtw_security.c
@@ -139,17 +139,11 @@ static __le32 getcrc32(u8 *buf, int len)
Need to consider the fragment situation
*/
void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe)
-{ /* exclude ICV */
-
- unsigned char crc[4];
- struct arc4context mycontext;
-
+{
int curfragnum, length;
- u32 keylength;
- u8 *pframe, *payload, *iv; /* wepkey */
- u8 wepkey[16];
- u8 hw_hdr_offset = 0;
+ u8 *pframe;
+ u8 hw_hdr_offset = 0;
struct pkt_attrib *pattrib = &((struct xmit_frame *)pxmitframe)->attrib;
struct security_priv *psecuritypriv = &padapter->securitypriv;
struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
@@ -165,33 +159,53 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe)
/* start to encrypt each fragment */
if ((pattrib->encrypt == _WEP40_) || (pattrib->encrypt == _WEP104_)) {
- keylength = psecuritypriv->dot11DefKeylen[psecuritypriv->dot11PrivacyKeyIndex];
+ const int keyindex = psecuritypriv->dot11PrivacyKeyIndex;
+ void *crypto_private;
+ struct sk_buff *skb;
+ struct lib80211_crypto_ops *crypto_ops = try_then_request_module(lib80211_get_crypto_ops("WEP"), "lib80211_crypt_wep");
+
+ if (!crypto_ops)
+ goto exit;
+
+ crypto_private = crypto_ops->init(keyindex);
+ if (!crypto_private)
+ goto exit;
+
+ if (crypto_ops->set_key(psecuritypriv->dot11DefKey[keyindex].skey,
+ psecuritypriv->dot11DefKeylen[keyindex], NULL, crypto_private) < 0)
+ goto exit;
for (curfragnum = 0; curfragnum < pattrib->nr_frags; curfragnum++) {
- iv = pframe+pattrib->hdrlen;
- memcpy(&wepkey[0], iv, 3);
- memcpy(&wepkey[3], &psecuritypriv->dot11DefKey[psecuritypriv->dot11PrivacyKeyIndex].skey[0], keylength);
- payload = pframe+pattrib->iv_len+pattrib->hdrlen;
+ if (curfragnum + 1 == pattrib->nr_frags)
+ length = pattrib->last_txcmdsz;
+ else
+ length = pxmitpriv->frag_len;
+ skb = dev_alloc_skb(length);
+ if (!skb)
+ goto exit;
- if ((curfragnum+1) == pattrib->nr_frags) { /* the last fragment */
- length = pattrib->last_txcmdsz-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len;
+ skb_put_data(skb, pframe, length);
- *((__le32 *)crc) = getcrc32(payload, length);
+ memmove(skb->data + 4, skb->data, pattrib->hdrlen);
+ skb_pull(skb, 4);
+ skb_trim(skb, skb->len - 4);
- arcfour_init(&mycontext, wepkey, 3+keylength);
- arcfour_encrypt(&mycontext, payload, payload, length);
- arcfour_encrypt(&mycontext, payload+length, crc, 4);
- } else {
- length = pxmitpriv->frag_len-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len;
- *((__le32 *)crc) = getcrc32(payload, length);
- arcfour_init(&mycontext, wepkey, 3+keylength);
- arcfour_encrypt(&mycontext, payload, payload, length);
- arcfour_encrypt(&mycontext, payload+length, crc, 4);
-
- pframe += pxmitpriv->frag_len;
- pframe = (u8 *)round_up((size_t)(pframe), 4);
+ if (crypto_ops->encrypt_mpdu(skb, pattrib->hdrlen, crypto_private)) {
+ kfree_skb(skb);
+ goto exit;
}
+
+ memcpy(pframe, skb->data, skb->len);
+
+ pframe += skb->len;
+ pframe = (u8 *)round_up((size_t)(pframe), 4);
+
+ kfree_skb(skb);
}
+
+exit:
+ if (crypto_ops && crypto_private)
+ crypto_ops->deinit(crypto_private);
}
}
--
2.16.1
On Mon, May 28, 2018 at 09:18:21AM +0300, Ivan Safonov wrote:
> Put data to skb, decrypt with lib80211_crypt_wep, and place back to tx buffer.
>
> Signed-off-by: Ivan Safonov <[email protected]>
> ---
> drivers/staging/rtl8188eu/core/rtw_security.c | 72 ++++++++++++++++-----------
> 1 file changed, 43 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/staging/rtl8188eu/core/rtw_security.c b/drivers/staging/rtl8188eu/core/rtw_security.c
> index bfe0b217e679..80d7569a3108 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_security.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_security.c
> @@ -139,17 +139,11 @@ static __le32 getcrc32(u8 *buf, int len)
> Need to consider the fragment situation
> */
> void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe)
> -{ /* exclude ICV */
> -
> - unsigned char crc[4];
> - struct arc4context mycontext;
> -
> +{
> int curfragnum, length;
> - u32 keylength;
>
> - u8 *pframe, *payload, *iv; /* wepkey */
> - u8 wepkey[16];
> - u8 hw_hdr_offset = 0;
> + u8 *pframe;
> + u8 hw_hdr_offset = 0;
> struct pkt_attrib *pattrib = &((struct xmit_frame *)pxmitframe)->attrib;
> struct security_priv *psecuritypriv = &padapter->securitypriv;
> struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
> @@ -165,33 +159,53 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe)
>
> /* start to encrypt each fragment */
> if ((pattrib->encrypt == _WEP40_) || (pattrib->encrypt == _WEP104_)) {
> - keylength = psecuritypriv->dot11DefKeylen[psecuritypriv->dot11PrivacyKeyIndex];
> + const int keyindex = psecuritypriv->dot11PrivacyKeyIndex;
> + void *crypto_private;
> + struct sk_buff *skb;
> + struct lib80211_crypto_ops *crypto_ops = try_then_request_module(lib80211_get_crypto_ops("WEP"), "lib80211_crypt_wep");
> +
> + if (!crypto_ops)
> + goto exit;
> +
> + crypto_private = crypto_ops->init(keyindex);
> + if (!crypto_private)
> + goto exit;
> +
> + if (crypto_ops->set_key(psecuritypriv->dot11DefKey[keyindex].skey,
> + psecuritypriv->dot11DefKeylen[keyindex], NULL, crypto_private) < 0)
> + goto exit;
>
> for (curfragnum = 0; curfragnum < pattrib->nr_frags; curfragnum++) {
> - iv = pframe+pattrib->hdrlen;
> - memcpy(&wepkey[0], iv, 3);
> - memcpy(&wepkey[3], &psecuritypriv->dot11DefKey[psecuritypriv->dot11PrivacyKeyIndex].skey[0], keylength);
> - payload = pframe+pattrib->iv_len+pattrib->hdrlen;
> + if (curfragnum + 1 == pattrib->nr_frags)
> + length = pattrib->last_txcmdsz;
> + else
> + length = pxmitpriv->frag_len;
> + skb = dev_alloc_skb(length);
> + if (!skb)
> + goto exit;
>
> - if ((curfragnum+1) == pattrib->nr_frags) { /* the last fragment */
> - length = pattrib->last_txcmdsz-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len;
> + skb_put_data(skb, pframe, length);
>
> - *((__le32 *)crc) = getcrc32(payload, length);
> + memmove(skb->data + 4, skb->data, pattrib->hdrlen);
> + skb_pull(skb, 4);
> + skb_trim(skb, skb->len - 4);
>
> - arcfour_init(&mycontext, wepkey, 3+keylength);
> - arcfour_encrypt(&mycontext, payload, payload, length);
> - arcfour_encrypt(&mycontext, payload+length, crc, 4);
> - } else {
> - length = pxmitpriv->frag_len-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len;
> - *((__le32 *)crc) = getcrc32(payload, length);
> - arcfour_init(&mycontext, wepkey, 3+keylength);
> - arcfour_encrypt(&mycontext, payload, payload, length);
> - arcfour_encrypt(&mycontext, payload+length, crc, 4);
> -
> - pframe += pxmitpriv->frag_len;
> - pframe = (u8 *)round_up((size_t)(pframe), 4);
> + if (crypto_ops->encrypt_mpdu(skb, pattrib->hdrlen, crypto_private)) {
> + kfree_skb(skb);
> + goto exit;
> }
> +
> + memcpy(pframe, skb->data, skb->len);
> +
> + pframe += skb->len;
> + pframe = (u8 *)round_up((size_t)(pframe), 4);
> +
> + kfree_skb(skb);
> }
> +
> +exit:
> + if (crypto_ops && crypto_private)
> + crypto_ops->deinit(crypto_private);
One label style error handling is always bugggy. I'm surprised GCC
doesn't catch that crypto_private can be uninitialized...
Flip the if ((pattrib->encrypt == _WEP40_) || (pattrib->encrypt == _WEP104_)) {
tests so it's:
if (pattrib->encrypt != _WEP40_ && pattrib->encrypt != _WEP104_)
return;
The use normal error handling style:
kfree_skb(skb);
return;
err_free_skb:
kfree_skb(skb);
err_deinit:
crypto_ops->deinit(crypto_private);
}
regards,
dan carpenter
On 05/28/2018 04:53 PM, Dan Carpenter wrote:
> On Mon, May 28, 2018 at 09:18:21AM +0300, Ivan Safonov wrote:
>> Put data to skb, decrypt with lib80211_crypt_wep, and place back to tx buffer.
>>
>> Signed-off-by: Ivan Safonov <[email protected]>
>> ---
>> drivers/staging/rtl8188eu/core/rtw_security.c | 72 ++++++++++++++++-----------
>> 1 file changed, 43 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/staging/rtl8188eu/core/rtw_security.c b/drivers/staging/rtl8188eu/core/rtw_security.c
>> index bfe0b217e679..80d7569a3108 100644
>> --- a/drivers/staging/rtl8188eu/core/rtw_security.c
>> +++ b/drivers/staging/rtl8188eu/core/rtw_security.c
>> @@ -139,17 +139,11 @@ static __le32 getcrc32(u8 *buf, int len)
>> Need to consider the fragment situation
>> */
>> void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe)
>> -{ /* exclude ICV */
>> -
>> - unsigned char crc[4];
>> - struct arc4context mycontext;
>> -
>> +{
>> int curfragnum, length;
>> - u32 keylength;
>>
>> - u8 *pframe, *payload, *iv; /* wepkey */
>> - u8 wepkey[16];
>> - u8 hw_hdr_offset = 0;
>> + u8 *pframe;
>> + u8 hw_hdr_offset = 0;
>> struct pkt_attrib *pattrib = &((struct xmit_frame *)pxmitframe)->attrib;
>> struct security_priv *psecuritypriv = &padapter->securitypriv;
>> struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
>> @@ -165,33 +159,53 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe)
>>
>> /* start to encrypt each fragment */
>> if ((pattrib->encrypt == _WEP40_) || (pattrib->encrypt == _WEP104_)) {
>> - keylength = psecuritypriv->dot11DefKeylen[psecuritypriv->dot11PrivacyKeyIndex];
>> + const int keyindex = psecuritypriv->dot11PrivacyKeyIndex;
>> + void *crypto_private;
>> + struct sk_buff *skb;
>> + struct lib80211_crypto_ops *crypto_ops = try_then_request_module(lib80211_get_crypto_ops("WEP"), "lib80211_crypt_wep");
>> +
>> + if (!crypto_ops)
>> + goto exit;
>> +
>> + crypto_private = crypto_ops->init(keyindex);
>> + if (!crypto_private)
>> + goto exit;
>> +
>> + if (crypto_ops->set_key(psecuritypriv->dot11DefKey[keyindex].skey,
>> + psecuritypriv->dot11DefKeylen[keyindex], NULL, crypto_private) < 0)
>> + goto exit;
>>
>> for (curfragnum = 0; curfragnum < pattrib->nr_frags; curfragnum++) {
>> - iv = pframe+pattrib->hdrlen;
>> - memcpy(&wepkey[0], iv, 3);
>> - memcpy(&wepkey[3], &psecuritypriv->dot11DefKey[psecuritypriv->dot11PrivacyKeyIndex].skey[0], keylength);
>> - payload = pframe+pattrib->iv_len+pattrib->hdrlen;
>> + if (curfragnum + 1 == pattrib->nr_frags)
>> + length = pattrib->last_txcmdsz;
>> + else
>> + length = pxmitpriv->frag_len;
>> + skb = dev_alloc_skb(length);
>> + if (!skb)
>> + goto exit;
>>
>> - if ((curfragnum+1) == pattrib->nr_frags) { /* the last fragment */
>> - length = pattrib->last_txcmdsz-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len;
>> + skb_put_data(skb, pframe, length);
>>
>> - *((__le32 *)crc) = getcrc32(payload, length);
>> + memmove(skb->data + 4, skb->data, pattrib->hdrlen);
>> + skb_pull(skb, 4);
>> + skb_trim(skb, skb->len - 4);
>>
>> - arcfour_init(&mycontext, wepkey, 3+keylength);
>> - arcfour_encrypt(&mycontext, payload, payload, length);
>> - arcfour_encrypt(&mycontext, payload+length, crc, 4);
>> - } else {
>> - length = pxmitpriv->frag_len-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len;
>> - *((__le32 *)crc) = getcrc32(payload, length);
>> - arcfour_init(&mycontext, wepkey, 3+keylength);
>> - arcfour_encrypt(&mycontext, payload, payload, length);
>> - arcfour_encrypt(&mycontext, payload+length, crc, 4);
>> -
>> - pframe += pxmitpriv->frag_len;
>> - pframe = (u8 *)round_up((size_t)(pframe), 4);
>> + if (crypto_ops->encrypt_mpdu(skb, pattrib->hdrlen, crypto_private)) {
>> + kfree_skb(skb);
>> + goto exit;
>> }
>> +
>> + memcpy(pframe, skb->data, skb->len);
>> +
>> + pframe += skb->len;
>> + pframe = (u8 *)round_up((size_t)(pframe), 4);
>> +
>> + kfree_skb(skb);
>> }
>> +
>> +exit:
>> + if (crypto_ops && crypto_private)
>> + crypto_ops->deinit(crypto_private);
>
> One label style error handling is always bugggy. I'm surprised GCC
> doesn't catch that crypto_private can be uninitialized...
This is not a compiler defect. The code
if (a && b) {
...
}
is equal to
if (a) {
if (b) {
...
}
}
>
> Flip the if ((pattrib->encrypt == _WEP40_) || (pattrib->encrypt == _WEP104_)) {
> tests so it's:
>
> if (pattrib->encrypt != _WEP40_ && pattrib->encrypt != _WEP104_)
> return;
The check is superfluous inside this function, it should be
(somewhen) around rtw_wep_encrypt().
>
> The use normal error handling style:
>
> kfree_skb(skb);
> return;
>
> err_free_skb:
> kfree_skb(skb);
> err_deinit:
> crypto_ops->deinit(crypto_private);
> }
>
rtw_(wep|aes|tkip)_(en|de)crypt still need to be rewritten, so I'll
leave this patch as is. In the following patches I'll take your comments
into account, they really simplify reading the code.
> regards,
> dan carpenter
>
Ivan Safonov.
On Sun, Jun 03, 2018 at 07:16:19PM +0300, Ivan Safonov wrote:
> On 05/28/2018 04:53 PM, Dan Carpenter wrote:
> > On Mon, May 28, 2018 at 09:18:21AM +0300, Ivan Safonov wrote:
> > > Put data to skb, decrypt with lib80211_crypt_wep, and place back to tx buffer.
> > >
> > > Signed-off-by: Ivan Safonov <[email protected]>
> > > ---
> > > drivers/staging/rtl8188eu/core/rtw_security.c | 72 ++++++++++++++++-----------
> > > 1 file changed, 43 insertions(+), 29 deletions(-)
> > >
> > > diff --git a/drivers/staging/rtl8188eu/core/rtw_security.c b/drivers/staging/rtl8188eu/core/rtw_security.c
> > > index bfe0b217e679..80d7569a3108 100644
> > > --- a/drivers/staging/rtl8188eu/core/rtw_security.c
> > > +++ b/drivers/staging/rtl8188eu/core/rtw_security.c
> > > @@ -139,17 +139,11 @@ static __le32 getcrc32(u8 *buf, int len)
> > > Need to consider the fragment situation
> > > */
> > > void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe)
> > > -{ /* exclude ICV */
> > > -
> > > - unsigned char crc[4];
> > > - struct arc4context mycontext;
> > > -
> > > +{
> > > int curfragnum, length;
> > > - u32 keylength;
> > > - u8 *pframe, *payload, *iv; /* wepkey */
> > > - u8 wepkey[16];
> > > - u8 hw_hdr_offset = 0;
> > > + u8 *pframe;
> > > + u8 hw_hdr_offset = 0;
> > > struct pkt_attrib *pattrib = &((struct xmit_frame *)pxmitframe)->attrib;
> > > struct security_priv *psecuritypriv = &padapter->securitypriv;
> > > struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
> > > @@ -165,33 +159,53 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe)
> > > /* start to encrypt each fragment */
> > > if ((pattrib->encrypt == _WEP40_) || (pattrib->encrypt == _WEP104_)) {
> > > - keylength = psecuritypriv->dot11DefKeylen[psecuritypriv->dot11PrivacyKeyIndex];
> > > + const int keyindex = psecuritypriv->dot11PrivacyKeyIndex;
> > > + void *crypto_private;
> > > + struct sk_buff *skb;
> > > + struct lib80211_crypto_ops *crypto_ops = try_then_request_module(lib80211_get_crypto_ops("WEP"), "lib80211_crypt_wep");
> > > +
> > > + if (!crypto_ops)
> > > + goto exit;
> > > +
> > > + crypto_private = crypto_ops->init(keyindex);
> > > + if (!crypto_private)
> > > + goto exit;
> > > +
> > > + if (crypto_ops->set_key(psecuritypriv->dot11DefKey[keyindex].skey,
> > > + psecuritypriv->dot11DefKeylen[keyindex], NULL, crypto_private) < 0)
> > > + goto exit;
> > > for (curfragnum = 0; curfragnum < pattrib->nr_frags; curfragnum++) {
> > > - iv = pframe+pattrib->hdrlen;
> > > - memcpy(&wepkey[0], iv, 3);
> > > - memcpy(&wepkey[3], &psecuritypriv->dot11DefKey[psecuritypriv->dot11PrivacyKeyIndex].skey[0], keylength);
> > > - payload = pframe+pattrib->iv_len+pattrib->hdrlen;
> > > + if (curfragnum + 1 == pattrib->nr_frags)
> > > + length = pattrib->last_txcmdsz;
> > > + else
> > > + length = pxmitpriv->frag_len;
> > > + skb = dev_alloc_skb(length);
> > > + if (!skb)
> > > + goto exit;
> > > - if ((curfragnum+1) == pattrib->nr_frags) { /* the last fragment */
> > > - length = pattrib->last_txcmdsz-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len;
> > > + skb_put_data(skb, pframe, length);
> > > - *((__le32 *)crc) = getcrc32(payload, length);
> > > + memmove(skb->data + 4, skb->data, pattrib->hdrlen);
> > > + skb_pull(skb, 4);
> > > + skb_trim(skb, skb->len - 4);
> > > - arcfour_init(&mycontext, wepkey, 3+keylength);
> > > - arcfour_encrypt(&mycontext, payload, payload, length);
> > > - arcfour_encrypt(&mycontext, payload+length, crc, 4);
> > > - } else {
> > > - length = pxmitpriv->frag_len-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len;
> > > - *((__le32 *)crc) = getcrc32(payload, length);
> > > - arcfour_init(&mycontext, wepkey, 3+keylength);
> > > - arcfour_encrypt(&mycontext, payload, payload, length);
> > > - arcfour_encrypt(&mycontext, payload+length, crc, 4);
> > > -
> > > - pframe += pxmitpriv->frag_len;
> > > - pframe = (u8 *)round_up((size_t)(pframe), 4);
> > > + if (crypto_ops->encrypt_mpdu(skb, pattrib->hdrlen, crypto_private)) {
> > > + kfree_skb(skb);
> > > + goto exit;
> > > }
> > > +
> > > + memcpy(pframe, skb->data, skb->len);
> > > +
> > > + pframe += skb->len;
> > > + pframe = (u8 *)round_up((size_t)(pframe), 4);
> > > +
> > > + kfree_skb(skb);
> > > }
> > > +
> > > +exit:
> > > + if (crypto_ops && crypto_private)
> > > + crypto_ops->deinit(crypto_private);
> >
> > One label style error handling is always bugggy. I'm surprised GCC
> > doesn't catch that crypto_private can be uninitialized...
>
> This is not a compiler defect. The code
>
> if (a && b) {
> ...
> }
>
> is equal to
>
> if (a) {
> if (b) {
> ...
> }
> }
Heh... Oops. Sorry, I misread the code. That's embarrassing.
> rtw_(wep|aes|tkip)_(en|de)crypt still need to be rewritten, so I'll leave
> this patch as is. In the following patches I'll take your comments into
> account, they really simplify reading the code.
>
Sounds good.
regards,
dan carpenter